From f4dd9130315b8461b4f836a6944c9cfd952dd1fb Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Wed, 20 Dec 2017 16:56:10 +0100 Subject: [PATCH 1/2] ebtables: Use flock() for --concurrent option The previous locking mechanism was not atomic, hence it was possible that a killed ebtables process would leave the lock file in place which in turn made future ebtables processes wait indefinitely for the lock to become free. Fix this by using flock(). This also simplifies code quite a bit because there is no need for a custom signal handler or an __exit routine anymore. Signed-off-by: Sven Eckelmann --- ...bles-Use-flock-for-concurrent-option.patch | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 patches/lede/0048-ebtables-Use-flock-for-concurrent-option.patch diff --git a/patches/lede/0048-ebtables-Use-flock-for-concurrent-option.patch b/patches/lede/0048-ebtables-Use-flock-for-concurrent-option.patch new file mode 100644 index 00000000..e95dd2ff --- /dev/null +++ b/patches/lede/0048-ebtables-Use-flock-for-concurrent-option.patch @@ -0,0 +1,146 @@ +From: Sven Eckelmann +Date: Wed, 20 Dec 2017 16:55:17 +0100 +Subject: ebtables: Use flock() for --concurrent option + +The previous locking mechanism was not atomic, hence it was possible +that a killed ebtables process would leave the lock file in place which +in turn made future ebtables processes wait indefinitely for the lock to +become free. + +Fix this by using flock(). This also simplifies code quite a bit because +there is no need for a custom signal handler or an __exit routine +anymore. + +diff --git a/package/network/utils/ebtables/patches/300-fix-concurrent.patch b/package/network/utils/ebtables/patches/300-fix-concurrent.patch +new file mode 100644 +index 0000000000000000000000000000000000000000..1a99162bf51cd175e26d49e7ee5277b8b8645f48 +--- /dev/null ++++ b/package/network/utils/ebtables/patches/300-fix-concurrent.patch +@@ -0,0 +1,127 @@ ++From 6a826591878db3fa9e2a94b87a3d5edd8e0fc442 Mon Sep 17 00:00:00 2001 ++From: Phil Sutter ++Date: Fri, 6 Oct 2017 12:48:50 +0200 ++Subject: Use flock() for --concurrent option ++ ++The previous locking mechanism was not atomic, hence it was possible ++that a killed ebtables process would leave the lock file in place which ++in turn made future ebtables processes wait indefinitely for the lock to ++become free. ++ ++Fix this by using flock(). This also simplifies code quite a bit because ++there is no need for a custom signal handler or an __exit routine ++anymore. ++ ++Signed-off-by: Phil Sutter ++Signed-off-by: Pablo Neira Ayuso ++ ++Origin: upstream, https://git.netfilter.org/ebtables/commit/?id=6a826591878db3fa9e2a94b87a3d5edd8e0fc442 ++--- ++ ebtables.c | 8 -------- ++ libebtc.c | 49 +++++-------------------------------------------- ++ 2 files changed, 5 insertions(+), 52 deletions(-) ++ ++diff --git a/ebtables.c b/ebtables.c ++index 62f1ba8..f7dfccf 100644 ++--- a/ebtables.c +++++ b/ebtables.c ++@@ -528,12 +528,6 @@ void ebt_early_init_once() ++ ebt_iterate_targets(merge_target); ++ } ++ ++-/* signal handler, installed when the option --concurrent is specified. */ ++-static void sighandler(int signum) ++-{ ++- exit(-1); ++-} ++- ++ /* We use exec_style instead of #ifdef's because ebtables.so is a shared object. */ ++ int do_command(int argc, char *argv[], int exec_style, ++ struct ebt_u_replace *replace_) ++@@ -1047,8 +1041,6 @@ big_iface_length: ++ strcpy(replace->filename, optarg); ++ break; ++ case 13 : /* concurrent */ ++- signal(SIGINT, sighandler); ++- signal(SIGTERM, sighandler); ++ use_lockfd = 1; ++ break; ++ case 1 : ++diff --git a/libebtc.c b/libebtc.c ++index 74830ec..c0ff8cc 100644 ++--- a/libebtc.c +++++ b/libebtc.c ++@@ -31,6 +31,7 @@ ++ #include "include/ethernetdb.h" ++ #include ++ #include +++#include ++ #include ++ #include ++ #include ++@@ -137,58 +138,18 @@ void ebt_list_extensions() ++ #define LOCKDIR "/var/lib/ebtables" ++ #define LOCKFILE LOCKDIR"/lock" ++ #endif ++-static int lockfd = -1, locked; ++ int use_lockfd; ++ /* Returns 0 on success, -1 when the file is locked by another process ++ * or -2 on any other error. */ ++ static int lock_file() ++ { ++- int try = 0; ++- int ret = 0; ++- sigset_t sigset; ++- ++-tryagain: ++- /* the SIGINT handler will call unlock_file. To make sure the state ++- * of the variable locked is correct, we need to temporarily mask the ++- * SIGINT interrupt. */ ++- sigemptyset(&sigset); ++- sigaddset(&sigset, SIGINT); ++- sigprocmask(SIG_BLOCK, &sigset, NULL); ++- lockfd = open(LOCKFILE, O_CREAT | O_EXCL | O_WRONLY, 00600); ++- if (lockfd < 0) { ++- if (errno == EEXIST) ++- ret = -1; ++- else if (try == 1) ++- ret = -2; ++- else { ++- if (mkdir(LOCKDIR, 00700)) ++- ret = -2; ++- else { ++- try = 1; ++- goto tryagain; ++- } ++- } ++- } else { ++- close(lockfd); ++- locked = 1; ++- } ++- sigprocmask(SIG_UNBLOCK, &sigset, NULL); ++- return ret; ++-} +++ int fd = open(LOCKFILE, O_CREAT, 00600); ++ ++-void unlock_file() ++-{ ++- if (locked) { ++- remove(LOCKFILE); ++- locked = 0; ++- } +++ if (fd < 0) +++ return -2; +++ return flock(fd, LOCK_EX); ++ } ++ ++-void __attribute__ ((destructor)) onexit() ++-{ ++- if (use_lockfd) ++- unlock_file(); ++-} ++ /* Get the table from the kernel or from a binary file ++ * init: 1 = ask the kernel for the initial contents of a table, i.e. the ++ * way it looks when the table is insmod'ed ++-- ++cgit v1.1 ++ From 1fc17fd63446ad5668dd42c1c0442c3136c606be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20L=C3=BCssing?= Date: Sun, 26 Nov 2017 22:40:02 +0100 Subject: [PATCH 2/2] gluon-ebtables: Enable concurrent ebtables updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This enables the ebtables internal locking mechanism which will avoid race conditions between multiple, concurrent ebtables calls. This is a preparation for the upcoming gluon-arp-limiter daemon, to avoid issues if upon restarting gluon-ebtables the gluon-arp-limiter daemon tries to modify the tables. Signed-off-by: Linus Lüssing --- .../gluon-ebtables/files/etc/init.d/gluon-ebtables | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/package/gluon-ebtables/files/etc/init.d/gluon-ebtables b/package/gluon-ebtables/files/etc/init.d/gluon-ebtables index e6bffe96..40c9e061 100755 --- a/package/gluon-ebtables/files/etc/init.d/gluon-ebtables +++ b/package/gluon-ebtables/files/etc/init.d/gluon-ebtables @@ -51,8 +51,12 @@ exec_all() { start() { ( - export EBTABLES_RULE='"ebtables -t " .. table .. " -A " .. command' - export EBTABLES_CHAIN='"ebtables -t " .. table .. " -N " .. name .. " -P " .. policy' + export EBTABLES_RULE='"ebtables --concurrent -t " .. table .. " -A " .. command' + export EBTABLES_CHAIN='"ebtables --concurrent -t " .. table .. " -N " .. name .. " -P " .. policy' + + # Contains /var/lib/ebtables/lock for '--concurrent' + [ ! -d "/var/lib/ebtables" ] && \ + mkdir -p /var/lib/ebtables if [ -z "$1" ]; then exec_all '' @@ -64,8 +68,8 @@ start() { stop() { ( - export EBTABLES_RULE='"ebtables -t " .. table .. " -D " .. command' - export EBTABLES_CHAIN='"ebtables -t " .. table .. " -X " .. name' + export EBTABLES_RULE='"ebtables --concurrent -t " .. table .. " -D " .. command' + export EBTABLES_CHAIN='"ebtables --concurrent -t " .. table .. " -X " .. name' if [ -z "$1" ]; then exec_all '-r'