Accepting request 506228 from home:mgerstner:branches:security:netfilter

- fix a locking issue of iptables-batch which can cause it to spuriously fail
  when other programs modify the iptables rules in parallel (bnc#1045130).
  This can especially affect SuSEfirewall2 during startup.

OBS-URL: https://build.opensuse.org/request/show/506228
OBS-URL: https://build.opensuse.org/package/show/security:netfilter/iptables?expand=0&rev=93

iptables-batch-lock.patch

@@ -0,0 +1,70 @@
+Index: iptables-1.6.1/iptables/iptables-batch.c
+===================================================================
+--- iptables-1.6.1.orig/iptables/iptables-batch.c
++++ iptables-1.6.1/iptables/iptables-batch.c
+@@ -404,6 +404,34 @@ main(int argc, char *argv[])
+ 	tables[3].handle = NULL;
+ 	current_table = &tables[0];
+ 
++	/*
++	 * we need to lock the complete batch processing against parallel
++	 * modification by other processes. Otherwise we can end up with
++	 * EAGAIN errors.
++	 *
++	 * the do_command{4,6} function already locks itself, but the
++	 * complete call sequence needs to be locked until the commit is
++	 * performed.
++	 *
++	 * sadly the xtables_lock() implementation is not very cooperative.
++	 * There's no unlock() equivalent. The lock file descriptor is smiply
++	 * left open until the process exits. Thus we'd have deadlocks when
++	 * calling do_command{4,6} the second time.
++	 *
++	 * To prevent this, part of this patch adds logic to avoid taking the
++	 * lock a second time in the same process in xtables_lock()
++	 */
++
++	const struct timeval wait_interval = {
++		.tv_sec = 1,
++	};
++
++	if( xtables_lock(-1, &wait_interval) != true )
++	{
++		fprintf(stderr, "failed to acquire the xtables lock\n");
++		exit(1);
++	}
++
+ 	while((r = getline(&iline, &llen, fp)) != -1)
+ 	{
+ 		if(llen < 1 || !*iline)
+Index: iptables-1.6.1/iptables/xshared.c
+===================================================================
+--- iptables-1.6.1.orig/iptables/xshared.c
++++ iptables-1.6.1/iptables/xshared.c
+@@ -250,8 +250,14 @@ void xs_init_match(struct xtables_match
+ bool xtables_lock(int wait, struct timeval *wait_interval)
+ {
+ 	struct timeval time_left, wait_time, waited_time;
++	static bool already_locked = false;
+ 	int fd, i = 0;
+ 
++	if( already_locked ) {
++		// avoid dead-locks, see iptables-batch.c
++		return true;
++	}
++
+ 	time_left.tv_sec = wait;
+ 	time_left.tv_usec = 0;
+ 	waited_time.tv_sec = 0;
+@@ -262,8 +268,10 @@ bool xtables_lock(int wait, struct timev
+ 		return true;
+ 
+ 	while (1) {
+-		if (flock(fd, LOCK_EX | LOCK_NB) == 0)
++		if (flock(fd, LOCK_EX | LOCK_NB) == 0) {
++			already_locked = true;
+ 			return true;
++		}
+ 		if (++i % 10 == 0) {
+ 			if (wait != -1)
+ 				fprintf(stderr, "Another app is currently holding the xtables lock; "

iptables.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Thu Jun 22 15:34:40 UTC 2017 - matthias.gerstner@suse.com
+
+- fix a locking issue of iptables-batch which can cause it to spuriously fail
+  when other programs modify the iptables rules in parallel (bnc#1045130).
+  This can especially affect SuSEfirewall2 during startup.
+
 -------------------------------------------------------------------
 Fri Jan 27 22:53:14 UTC 2017 - jengelh@inai.de
 

iptables.spec

@@ -28,6 +28,7 @@ Source2:        http://netfilter.org/projects/iptables/files/%name-%version.tar.
 Source3:        %name.keyring
 Patch3:         iptables-batch.patch
 Patch4:         iptables-apply-mktemp-fix.patch
+Patch5:         iptables-batch-lock.patch
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build
 %if 0%{?fedora_version} || 0%{?centos_version}
@@ -141,7 +142,7 @@ xtables --variable=xtlibdir).
 
 %prep
 %setup -q
-%patch -P 3 -P 4 -p1
+%patch -P 3 -P 4 -P 5 -p1
 
 %build
 # We have the iptables-batch patch, so always regenerate.