netlink: fix EILSEQ error messages due to process race condition

This patch fixes a race condition that triggers EILSEQ errors
(wrong sequence message). The problems is triggered when the child
process resets the timers at the same time that the parent process
requests a resync. Since both the child and the parent process use
the same descriptors, the sequence tracking code in libnfnetlink
gets confused as it considers that it is receiving out of sequence
netlink messages.

This patch introduces internal handlers to commit and reset timers
so that the parent and the child do not use the same descriptors
to operate with the kernel.

This patch changes the prototype of all nf_*_conntrack() functions.
Now, the nfct handler is passed as first parameter, this change is
required to fix this problem. The rest of the changes on the API
is done for consistency.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

1 files changed, 40 insertions, 16 deletions

diff --git a/src/cache_iterators.c b/src/cache_iterators.c
index fd7aed6..661528f 100644
--- a/src/cache_iterators.c
+++ b/src/cache_iterators.c
@@ -95,7 +95,13 @@ void cache_dump(struct cache *c, int fd, int type)
 	hashtable_iterate(c->h, (void *) &tmp, do_dump);
 }
 
-static void __do_commit_step(struct cache *c, struct us_conntrack *u)
+struct __commit_container {
+	struct nfct_handle 	*h;
+	struct cache 		*c;
+};
+
+static void
+__do_commit_step(struct __commit_container *tmp, struct us_conntrack *u)
 {
 	int ret, retry = 1;
 	struct nf_conntrack *ct = u->ct;
@@ -107,14 +113,14 @@ static void __do_commit_step(struct cache *c, struct us_conntrack *u)
 	nfct_set_attr_u32(ct, ATTR_TIMEOUT, CONFIG(commit_timeout));
 
 try_again:
-	ret = nl_exist_conntrack(ct);
+	ret = nl_exist_conntrack(tmp->h, ct);
 	switch (ret) {
 	case -1:
 		dlog(LOG_ERR, "commit-exist: %s", strerror(errno));
 		dlog_ct(STATE(log), ct, NFCT_O_PLAIN);
 		break;
 	case 0:
-		if (nl_create_conntrack(ct) == -1) {
+		if (nl_create_conntrack(tmp->h, ct) == -1) {
 			if (errno == ENOMEM) {
 				if (retry) {
 					retry = 0;
@@ -124,13 +130,13 @@ try_again:
 			}
 			dlog(LOG_ERR, "commit-create: %s", strerror(errno));
 			dlog_ct(STATE(log), ct, NFCT_O_PLAIN);
-			c->commit_fail++;
+			tmp->c->commit_fail++;
 		} else
-			c->commit_ok++;
+			tmp->c->commit_ok++;
 		break;
 	case 1:
-		c->commit_exist++;
-		if (nl_update_conntrack(ct) == -1) {
+		tmp->c->commit_exist++;
+		if (nl_update_conntrack(tmp->h, ct) == -1) {
 			if (errno == ENOMEM || errno == ETIME) {
 				if (retry) {
 					retry = 0;
@@ -140,7 +146,7 @@ try_again:
 			}
 			/* try harder, delete the entry and retry */
 			if (retry) {
-				ret = nl_destroy_conntrack(ct);
+				ret = nl_destroy_conntrack(tmp->h, ct);
 				if (ret == 0 || 
 				    (ret == -1 && errno == ENOENT)) {
 					retry = 0;
@@ -148,14 +154,14 @@ try_again:
 				}
 				dlog(LOG_ERR, "commit-rm: %s", strerror(errno));
 				dlog_ct(STATE(log), ct, NFCT_O_PLAIN);
-				c->commit_fail++;
+				tmp->c->commit_fail++;
 				break;
 			} 
 			dlog(LOG_ERR, "commit-update: %s", strerror(errno));
 			dlog_ct(STATE(log), ct, NFCT_O_PLAIN);
-			c->commit_fail++;
+			tmp->c->commit_fail++;
 		} else
-			c->commit_ok++;
+			tmp->c->commit_ok++;
 		break;
 	}
 }
@@ -188,10 +194,18 @@ void cache_commit(struct cache *c)
 	unsigned int commit_ok = c->commit_ok;
 	unsigned int commit_exist = c->commit_exist;
 	unsigned int commit_fail = c->commit_fail;
+	struct __commit_container tmp;
+
+	tmp.h = nfct_open(CONNTRACK, 0);
+	if (tmp.h == NULL) {
+		dlog(LOG_ERR, "can't create handler to commit entries");
+		return;
+	}
+	tmp.c = c;
 
 	/* commit master conntrack first, then related ones */
-	hashtable_iterate(c->h, c, do_commit_master);
-	hashtable_iterate(c->h, c, do_commit_related);
+	hashtable_iterate(c->h, &tmp, do_commit_master);
+	hashtable_iterate(c->h, &tmp, do_commit_related);
 
 	/* calculate new entries committed */
 	commit_ok = c->commit_ok - commit_ok;
@@ -207,16 +221,18 @@ void cache_commit(struct cache *c)
 	if (commit_fail)
 		dlog(LOG_NOTICE, "%u entries can't be "
 				 "committed", commit_fail);
+	nfct_close(tmp.h);
 }
 
 static int do_reset_timers(void *data1, void *data2)
 {
 	int ret;
 	u_int32_t current_timeout;
+	struct nfct_handle *h = data1;
 	struct us_conntrack *u = data2;
 	struct nf_conntrack *ct = u->ct;
 
-	ret = nl_get_conntrack(ct);
+	ret = nl_get_conntrack(h, ct);
 	switch (ret) {
 	case -1:
 		/* the kernel table is not in sync with internal cache */
@@ -231,7 +247,7 @@ static int do_reset_timers(void *data1, void *data2)
 
 		nfct_set_attr_u32(ct, ATTR_TIMEOUT, CONFIG(purge_timeout));
 
-		if (nl_update_conntrack(ct) == -1) {
+		if (nl_update_conntrack(h, ct) == -1) {
 			if (errno == ETIME || errno == ENOENT)
 				break;
 			dlog(LOG_ERR, "reset-timers-upd: %s", strerror(errno));
@@ -244,7 +260,15 @@ static int do_reset_timers(void *data1, void *data2)
 
 void cache_reset_timers(struct cache *c)
 {
-	hashtable_iterate(c->h, NULL, do_reset_timers);
+	struct nfct_handle *h;
+
+	h = nfct_open(CONNTRACK, 0);
+	if (h == NULL) {
+		dlog(LOG_ERR, "can't create handler to reset timers");
+		return;
+	}
+	hashtable_iterate(c->h, h, do_reset_timers);
+	nfct_close(h);
 }
 
 static int do_flush(void *data1, void *data2)