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>

4 files changed, 66 insertions, 42 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)
diff --git a/src/cache_wt.c b/src/cache_wt.c
index 65a1fc4..d0ae8bb 100644
--- a/src/cache_wt.c
+++ b/src/cache_wt.c
@@ -31,7 +31,7 @@ static void add_wt(struct us_conntrack *u)
 	char __ct[nfct_maxsize()];
 	struct nf_conntrack *ct = (struct nf_conntrack *)(void*) __ct;
 
-	ret = nl_exist_conntrack(u->ct);
+	ret = nl_exist_conntrack(STATE(request), u->ct);
 	switch (ret) {
 	case -1:
 		dlog(LOG_ERR, "cache_wt problem: %s", strerror(errno));
@@ -39,14 +39,14 @@ static void add_wt(struct us_conntrack *u)
 		break;
 	case 0:
 		memcpy(ct, u->ct, nfct_maxsize());
-		if (nl_create_conntrack(ct) == -1) {
+		if (nl_create_conntrack(STATE(dump), ct) == -1) {
 			dlog(LOG_ERR, "cache_wt create: %s", strerror(errno));
 			dlog_ct(STATE(log), u->ct, NFCT_O_PLAIN);
 		}
 		break;
 	case 1:
 		memcpy(ct, u->ct, nfct_maxsize());
-		if (nl_update_conntrack(ct) == -1) {
+		if (nl_update_conntrack(STATE(dump), ct) == -1) {
 			dlog(LOG_ERR, "cache_wt crt-upd: %s", strerror(errno));
 			dlog_ct(STATE(log), u->ct, NFCT_O_PLAIN);
 		}
@@ -61,7 +61,7 @@ static void upd_wt(struct us_conntrack *u)
 
 	memcpy(ct, u->ct, nfct_maxsize());
 
-	if (nl_update_conntrack(ct) == -1) {
+	if (nl_update_conntrack(STATE(dump), ct) == -1) {
 		dlog(LOG_ERR, "cache_wt update:%s", strerror(errno));
 		dlog_ct(STATE(log), u->ct, NFCT_O_PLAIN);
 	}
@@ -79,7 +79,7 @@ static void writethrough_update(struct us_conntrack *u, void *data)
 
 static void writethrough_destroy(struct us_conntrack *u, void *data)
 {
-	nl_destroy_conntrack(u->ct);
+	nl_destroy_conntrack(STATE(dump), u->ct);
 }
 
 struct cache_feature writethrough_feature = {
diff --git a/src/netlink.c b/src/netlink.c
index 9d155aa..29281f4 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -143,20 +143,20 @@ void nl_resize_socket_buffer(struct nfct_handle *h)
 			 CONFIG(netlink_buffer_size));
 }
 
-int nl_dump_conntrack_table(void)
+int nl_dump_conntrack_table(struct nfct_handle *h)
 {
-	return nfct_query(STATE(dump), NFCT_Q_DUMP, &CONFIG(family));
+	return nfct_query(h, NFCT_Q_DUMP, &CONFIG(family));
 }
 
-int nl_flush_conntrack_table(void)
+int nl_flush_conntrack_table(struct nfct_handle *h)
 {
-	return nfct_query(STATE(request), NFCT_Q_FLUSH, &CONFIG(family));
+	return nfct_query(h, NFCT_Q_FLUSH, &CONFIG(family));
 }
 
-int nl_overrun_request_resync(void)
+int nl_overrun_request_resync(struct nfct_handle *h)
 {
 	int family = CONFIG(family);
-	return nfct_send(STATE(overrun), NFCT_Q_DUMP, &family);
+	return nfct_send(h, NFCT_Q_DUMP, &family);
 }
 
 static int
@@ -178,18 +178,18 @@ __nl_get_conntrack(struct nfct_handle *h, const struct nf_conntrack *ct)
 	return 1;
 }
 
-int nl_exist_conntrack(const struct nf_conntrack *ct)
+int nl_exist_conntrack(struct nfct_handle *h, const struct nf_conntrack *ct)
 {
-	return __nl_get_conntrack(STATE(request), ct);
+	return __nl_get_conntrack(h, ct);
 }
 
 /* get the conntrack and update the cache */
-int nl_get_conntrack(const struct nf_conntrack *ct)
+int nl_get_conntrack(struct nfct_handle *h, const struct nf_conntrack *ct)
 {
-	return __nl_get_conntrack(STATE(dump), ct);
+	return __nl_get_conntrack(h, ct);
 }
 
-int nl_create_conntrack(const struct nf_conntrack *orig)
+int nl_create_conntrack(struct nfct_handle *h, const struct nf_conntrack *orig)
 {
 	int ret;
 	uint8_t flags;
@@ -217,13 +217,13 @@ int nl_create_conntrack(const struct nf_conntrack *orig)
 	nfct_set_attr_u8(ct, ATTR_TCP_FLAGS_REPL, flags);
 	nfct_set_attr_u8(ct, ATTR_TCP_MASK_REPL, flags);
 
-	ret = nfct_query(STATE(dump), NFCT_Q_CREATE, ct);
+	ret = nfct_query(h, NFCT_Q_CREATE, ct);
 	nfct_destroy(ct);
 
 	return ret;
 }
 
-int nl_update_conntrack(const struct nf_conntrack *orig)
+int nl_update_conntrack(struct nfct_handle *h, const struct nf_conntrack *orig)
 {
 	int ret;
 	uint8_t flags;
@@ -271,13 +271,13 @@ int nl_update_conntrack(const struct nf_conntrack *orig)
 	nfct_set_attr_u8(ct, ATTR_TCP_FLAGS_REPL, flags);
 	nfct_set_attr_u8(ct, ATTR_TCP_MASK_REPL, flags);
 
-	ret = nfct_query(STATE(dump), NFCT_Q_UPDATE, ct);
+	ret = nfct_query(h, NFCT_Q_UPDATE, ct);
 	nfct_destroy(ct);
 
 	return ret;
 }
 
-int nl_destroy_conntrack(const struct nf_conntrack *ct)
+int nl_destroy_conntrack(struct nfct_handle *h, const struct nf_conntrack *ct)
 {
-	return nfct_query(STATE(dump), NFCT_Q_DESTROY, ct);
+	return nfct_query(h, NFCT_Q_DESTROY, ct);
 }
diff --git a/src/run.c b/src/run.c
index 8158f10..ee985f4 100644
--- a/src/run.c
+++ b/src/run.c
@@ -111,11 +111,11 @@ void local_handler(int fd, void *data)
 	switch(type) {
 	case FLUSH_MASTER:
 		dlog(LOG_NOTICE, "flushing kernel conntrack table");
-		nl_flush_conntrack_table();
+		nl_flush_conntrack_table(STATE(request));
 		return;
 	case RESYNC_MASTER:
 		dlog(LOG_NOTICE, "resync with master table");
-		nl_dump_conntrack_table();
+		nl_dump_conntrack_table(STATE(dump));
 		return;
 	}
 
@@ -125,7 +125,7 @@ void local_handler(int fd, void *data)
 
 static void do_overrun_alarm(struct alarm_block *a, void *data)
 {
-	nl_overrun_request_resync();
+	nl_overrun_request_resync(STATE(overrun));
 	add_alarm(&STATE(overrun_alarm), 2, 0);
 }
 
@@ -218,7 +218,7 @@ init(void)
 	}
 	nfct_callback_register(STATE(dump), NFCT_T_ALL, dump_handler, NULL);
 
-	if (nl_dump_conntrack_table() == -1) {
+	if (nl_dump_conntrack_table(STATE(dump)) == -1) {
 		dlog(LOG_ERR, "can't get kernel conntrack table");
 		return -1;
 	}
@@ -321,7 +321,7 @@ static void __run(struct timeval *next_alarm)
 				 * size and resync with master conntrack table.
 				 */
 				nl_resize_socket_buffer(STATE(event));
-				nl_overrun_request_resync();
+				nl_overrun_request_resync(STATE(overrun));
 				add_alarm(&STATE(overrun_alarm), 2, 0);
 				break;
 			case ENOENT: