[PATCH nft] mnl: remove nft_mnl_socket_reopen()

[PATCH nft] mnl: remove nft_mnl_socket_reopen()

[Pablo Neira Ayuso]

nft_mnl_socket_reopen() was introduced to deal with the EINTR case.
By reopening the netlink socket, pending netlink messages that are part
part of a stale netlink dump are implicitly drop. This patch replaces
the nft_mnl_socket_reopen() strategy by pulling out all of the remaining
netlink message to restart in a clean state.

This is implicitly fixing up a bug in the table ownership support, which
assumes that the netlink socket remains open until nft_ctx_free() is
invoked.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/mnl.h |  1 -
 src/mnl.c     | 20 +++++++++++---------
 src/rule.c    |  5 ++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/mnl.h b/include/mnl.h
index 74b1b56fd686..979929c31c17 100644
--- a/include/mnl.h
+++ b/include/mnl.h
@@ -7,7 +7,6 @@
 #include <libmnl/libmnl.h>
 
 struct mnl_socket *nft_mnl_socket_open(void);
-struct mnl_socket *nft_mnl_socket_reopen(struct mnl_socket *nf_sock);
 
 uint32_t mnl_seqnum_alloc(uint32_t *seqnum);
 uint32_t mnl_genid_get(struct netlink_ctx *ctx);
diff --git a/src/mnl.c b/src/mnl.c
index 84cfb2380f55..2ae69c6f185e 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -52,13 +52,6 @@ struct mnl_socket *nft_mnl_socket_open(void)
 	return nf_sock;
 }
 
-struct mnl_socket *nft_mnl_socket_reopen(struct mnl_socket *nf_sock)
-{
-	mnl_socket_close(nf_sock);
-
-	return nft_mnl_socket_open();
-}
-
 uint32_t mnl_seqnum_alloc(unsigned int *seqnum)
 {
 	return (*seqnum)++;
@@ -77,17 +70,26 @@ nft_mnl_recv(struct netlink_ctx *ctx, uint32_t portid,
 	     int (*cb)(const struct nlmsghdr *nlh, void *data), void *cb_data)
 {
 	char buf[NFT_NLMSG_MAXSIZE];
+	bool eintr = false;
 	int ret;
 
 	ret = mnl_socket_recvfrom(ctx->nft->nf_sock, buf, sizeof(buf));
 	while (ret > 0) {
 		ret = mnl_cb_run(buf, ret, ctx->seqnum, portid, cb, cb_data);
-		if (ret <= 0)
-			goto out;
+		if (ret <= 0) {
+			if (errno != EINTR)
+				goto out;
+
+			eintr = true;
+		}
 
 		ret = mnl_socket_recvfrom(ctx->nft->nf_sock, buf, sizeof(buf));
 	}
 out:
+	if (eintr) {
+		ret = -1;
+		errno = EINTR;
+	}
 	if (ret < 0 && errno == EAGAIN)
 		return 0;
 
diff --git a/src/rule.c b/src/rule.c
index acb10f65a517..367c5c8be952 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -292,10 +292,9 @@ replay:
 	ret = cache_init(&ctx, flags);
 	if (ret < 0) {
 		cache_release(cache);
-		if (errno == EINTR) {
-			nft->nf_sock = nft_mnl_socket_reopen(nft->nf_sock);
+		if (errno == EINTR)
 			goto replay;
-		}
+
 		return -1;
 	}
 
-- 
2.20.1

[PATCH nft] cache: memleak list of chain

[Pablo Neira Ayuso]

Release chain list from the error path.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index 367c5c8be952..cf4d2cbef27b 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -170,32 +170,42 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
 		if (flags & NFT_CACHE_SET_BIT) {
 			ret = netlink_list_sets(ctx, &table->handle);
 			list_splice_tail_init(&ctx->list, &table->sets);
-			if (ret < 0)
-				return -1;
+			if (ret < 0) {
+				ret = -1;
+				goto cache_fails;
+			}
 		}
 		if (flags & NFT_CACHE_SETELEM_BIT) {
 			list_for_each_entry(set, &table->sets, list) {
 				ret = netlink_list_setelems(ctx, &set->handle,
 							    set);
-				if (ret < 0)
-					return -1;
+				if (ret < 0) {
+					ret = -1;
+					goto cache_fails;
+				}
 			}
 		}
 		if (flags & NFT_CACHE_CHAIN_BIT) {
 			ret = chain_cache_init(ctx, table, chain_list);
-			if (ret < 0)
-				return -1;
+			if (ret < 0) {
+				ret = -1;
+				goto cache_fails;
+			}
 		}
 		if (flags & NFT_CACHE_FLOWTABLE_BIT) {
 			ret = netlink_list_flowtables(ctx, &table->handle);
-			if (ret < 0)
-				return -1;
+			if (ret < 0) {
+				ret = -1;
+				goto cache_fails;
+			}
 			list_splice_tail_init(&ctx->list, &table->flowtables);
 		}
 		if (flags & NFT_CACHE_OBJECT_BIT) {
 			ret = netlink_list_objs(ctx, &table->handle);
-			if (ret < 0)
-				return -1;
+			if (ret < 0) {
+				ret = -1;
+				goto cache_fails;
+			}
 			list_splice_tail_init(&ctx->list, &table->objs);
 		}
 
@@ -208,15 +218,18 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
 							rule->handle.chain.name);
 				list_move_tail(&rule->list, &chain->rules);
 			}
-			if (ret < 0)
-				return -1;
+			if (ret < 0) {
+				ret = -1;
+				goto cache_fails;
+			}
 		}
 	}
 
+cache_fails:
 	if (flags & NFT_CACHE_CHAIN_BIT)
 		nftnl_chain_list_free(chain_list);
 
-	return 0;
+	return ret;
 }
 
 static int cache_init(struct netlink_ctx *ctx, unsigned int flags)
-- 
2.20.1