[PATCH nft 1/3] main: propagate error to shell

[PATCH nft 1/3] main: propagate error to shell

[Pablo Neira Ayuso]

Before:

 # nft add rule ip test input ip hdrlength 3
 <cmdline>:1:1-37: Error: Could not process rule: Invalid argument
 add rule ip test input ip hdrlength 3
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 # echo $?
 0

After:

 # nft add rule ip test input ip hdrlength 3
 <cmdline>:1:1-37: Error: Could not process rule: Invalid argument
 add rule ip test input ip hdrlength 3
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 # echo $?
 1

Reported-by: Ana Rey Botello <anarey@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/main.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/main.c b/src/main.c
index bd8feee..04a98e3 100644
--- a/src/main.c
+++ b/src/main.c
@@ -200,6 +200,8 @@ static int nft_netlink(struct parser_state *state, struct list_head *msgs)
 				netlink_io_error(&ctx, &cmd->location,
 						 "Could not process rule: %s",
 						 strerror(err->err));
+				ret = -1;
+				errno = err->err;
 				if (err->seqnum == cmd->seqnum) {
 					mnl_err_list_free(err);
 					break;
-- 
1.7.10.4

[PATCH nft 2/3] mnl: introduce NFT_NLMSG_MAXSIZE

[Pablo Neira Ayuso]

The NFT_NLMSG_MAXSIZE constant defines the maximum nf_tables netlink
message. Currently, the largest is the set element message, which
contains the NFTA_SET_ELEM_LIST_ELEMENTS attribute. This attribute is
a nest that describe the set elements. Given that the netlink attribute
length (nla_len) is 16 bits, the largest message is a bit larger than
64 KBytes. Thus, the proposed value of NFT_NLMSG_MAXSIZE is set to
(1 << 16) + getpagesize().

This new constant is used to calculate the length of:

1) the batch page length, when the batching mode is used.

2) the buffer that stores the netlink message in the send (when no
   batching is used) and receive paths.

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

diff --git a/src/mnl.c b/src/mnl.c
index 42b11f5..667aa3a 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -33,11 +33,19 @@ uint32_t mnl_seqnum_alloc(void)
 	return seq++;
 }
 
+/* The largest nf_tables netlink message is the set element message, which
+ * contains the NFTA_SET_ELEM_LIST_ELEMENTS attribute. This attribute is
+ * a nest that describes the set elements. Given that the netlink attribute
+ * length (nla_len) is 16 bits, the largest message is a bit larger than
+ * 64 KBytes.
+ */
+#define NFT_NLMSG_MAXSIZE (UINT16_MAX + getpagesize())
+
 static int
 nft_mnl_recv(struct mnl_socket *nf_sock, uint32_t seqnum, uint32_t portid,
 	     int (*cb)(const struct nlmsghdr *nlh, void *data), void *cb_data)
 {
-	char buf[MNL_SOCKET_BUFFER_SIZE];
+	char buf[NFT_NLMSG_MAXSIZE];
 	int ret;
 
 	ret = mnl_socket_recvfrom(nf_sock, buf, sizeof(buf));
@@ -86,8 +94,8 @@ static struct mnl_nlmsg_batch *mnl_batch_alloc(void)
 {
 	static char *buf;
 
-	/* libmnl needs higher buffer to handle batch overflows */
-	buf = xmalloc(BATCH_PAGE_SIZE + getpagesize());
+	/* libmnl needs higher buffer to handle batch overflows. */
+	buf = xmalloc(BATCH_PAGE_SIZE + NFT_NLMSG_MAXSIZE);
 	return mnl_nlmsg_batch_start(buf, BATCH_PAGE_SIZE);
 }
 
@@ -818,7 +826,7 @@ int mnl_nft_set_get(struct mnl_socket *nf_sock, struct nft_set *nls)
 int mnl_nft_setelem_add(struct mnl_socket *nf_sock, struct nft_set *nls,
 			unsigned int flags)
 {
-	char buf[MNL_SOCKET_BUFFER_SIZE];
+	char buf[NFT_NLMSG_MAXSIZE];
 	struct nlmsghdr *nlh;
 
 	nlh = nft_set_elem_nlmsg_build_hdr(buf, NFT_MSG_NEWSETELEM,
@@ -832,7 +840,7 @@ int mnl_nft_setelem_add(struct mnl_socket *nf_sock, struct nft_set *nls,
 int mnl_nft_setelem_delete(struct mnl_socket *nf_sock, struct nft_set *nls,
 			   unsigned int flags)
 {
-	char buf[MNL_SOCKET_BUFFER_SIZE];
+	char buf[NFT_NLMSG_MAXSIZE];
 	struct nlmsghdr *nlh;
 
 	nlh = nft_set_elem_nlmsg_build_hdr(buf, NFT_MSG_DELSETELEM,
-- 
1.7.10.4

[PATCH nft 3/3] mnl: fix crashes when using sets with many elements

[Pablo Neira Ayuso]

nft crashes when adding many elements into a set for two reasons:

1) The overflow of the nla_len field for the NFTA_SET_ELEM_LIST_ELEMENTS
   attribute.

2) Out-of-bound memory writes to the reserved area for the netlink
   message, which is solved by the patch entitled ("mnl: introduce
   NFT_NLMSG_MAXSIZE").

This patch adds the corresponding nla_len overflow check for
NFTA_SET_ELEM_LIST_ELEMENTS and it splits the elements in several
netlink messages. This should be enough when set updates are handled
by the transaction infrastructure

With this patch, nft should be now capable of adding an unlimited
number of elements to a given set.

Fixes: https://bugzilla.netfilter.org/show_bug.cgi?id=898
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/mnl.c |   43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/src/mnl.c b/src/mnl.c
index 667aa3a..febf7c2 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -828,13 +828,24 @@ int mnl_nft_setelem_add(struct mnl_socket *nf_sock, struct nft_set *nls,
 {
 	char buf[NFT_NLMSG_MAXSIZE];
 	struct nlmsghdr *nlh;
+	struct nft_set_elems_iter *iter;
+	int ret, err;
 
-	nlh = nft_set_elem_nlmsg_build_hdr(buf, NFT_MSG_NEWSETELEM,
-			nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY),
-			NLM_F_CREATE | NLM_F_ACK | flags, seq);
-	nft_set_elems_nlmsg_build_payload(nlh, nls);
+	iter = nft_set_elems_iter_create(nls);
+	if (iter == NULL)
+		memory_allocation_error();
 
-	return nft_mnl_talk(nf_sock, nlh, nlh->nlmsg_len, NULL, NULL);
+	do {
+		nlh = nft_set_elem_nlmsg_build_hdr(buf, NFT_MSG_NEWSETELEM,
+				nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY),
+				NLM_F_CREATE | NLM_F_ACK | flags, seq);
+		ret = nft_set_elems_nlmsg_build_payload_iter(nlh, iter);
+		err = nft_mnl_talk(nf_sock, nlh, nlh->nlmsg_len, NULL, NULL);
+	} while (ret > 0 && err >= 0);
+
+	nft_set_elems_iter_destroy(iter);
+
+	return err;
 }
 
 int mnl_nft_setelem_delete(struct mnl_socket *nf_sock, struct nft_set *nls,
@@ -861,13 +872,23 @@ int mnl_nft_setelem_batch_add(struct mnl_socket *nf_sock, struct nft_set *nls,
 			      unsigned int flags, uint32_t seqnum)
 {
 	struct nlmsghdr *nlh;
+	struct nft_set_elems_iter *iter;
+	int ret;
 
-	nlh = nft_set_elem_nlmsg_build_hdr(nft_nlmsg_batch_current(),
-			NFT_MSG_NEWSETELEM,
-			nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY),
-			NLM_F_CREATE | flags, seqnum);
-	nft_set_elems_nlmsg_build_payload(nlh, nls);
-	nft_batch_continue();
+	iter = nft_set_elems_iter_create(nls);
+	if (iter == NULL)
+		memory_allocation_error();
+
+	do {
+		nlh = nft_set_elem_nlmsg_build_hdr(nft_nlmsg_batch_current(),
+				NFT_MSG_NEWSETELEM,
+				nft_set_attr_get_u32(nls, NFT_SET_ATTR_FAMILY),
+				NLM_F_CREATE | flags, seqnum);
+		ret = nft_set_elems_nlmsg_build_payload_iter(nlh, iter);
+		nft_batch_continue();
+	} while (ret > 0);
+
+	nft_set_elems_iter_destroy(iter);
 
 	return 0;
 }
-- 
1.7.10.4