netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: davem@davemloft.net, netdev@vger.kernel.org
Subject: [PATCH 4/5] netfilter: nfnetlink: deliver netlink errors on batch completion
Date: Tue, 23 Sep 2014 11:24:47 +0200	[thread overview]
Message-ID: <1411464288-18069-5-git-send-email-pablo@netfilter.org> (raw)
In-Reply-To: <1411464288-18069-1-git-send-email-pablo@netfilter.org>

We have to wait until the full batch has been processed to deliver the
netlink error messages to userspace. Otherwise, we may deliver
duplicated errors to userspace in case that we need to abort and replay
the transaction if any of the required modules needs to be autoloaded.

A simple way to reproduce this (assumming nft_meta is not loaded) with
the following test file:

 add table filter
 add chain filter test
 add chain bad test                 # intentional wrong unexistent table
 add rule filter test meta mark 0

Then, when trying to load the batch:

 # nft -f test
 test:4:1-19: Error: Could not process rule: No such file or directory
 add chain bad test
 ^^^^^^^^^^^^^^^^^^^
 test:4:1-19: Error: Could not process rule: No such file or directory
 add chain bad test
 ^^^^^^^^^^^^^^^^^^^

The error is reported twice, once when the batch is aborted due to
missing nft_meta and another when it is fully processed.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink.c |   64 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index c138b8f..f37f071 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -222,6 +222,51 @@ replay:
 	}
 }
 
+struct nfnl_err {
+	struct list_head	head;
+	struct nlmsghdr		*nlh;
+	int			err;
+};
+
+static int nfnl_err_add(struct list_head *list, struct nlmsghdr *nlh, int err)
+{
+	struct nfnl_err *nfnl_err;
+
+	nfnl_err = kmalloc(sizeof(struct nfnl_err), GFP_KERNEL);
+	if (nfnl_err == NULL)
+		return -ENOMEM;
+
+	nfnl_err->nlh = nlh;
+	nfnl_err->err = err;
+	list_add_tail(&nfnl_err->head, list);
+
+	return 0;
+}
+
+static void nfnl_err_del(struct nfnl_err *nfnl_err)
+{
+	list_del(&nfnl_err->head);
+	kfree(nfnl_err);
+}
+
+static void nfnl_err_reset(struct list_head *err_list)
+{
+	struct nfnl_err *nfnl_err, *next;
+
+	list_for_each_entry_safe(nfnl_err, next, err_list, head)
+		nfnl_err_del(nfnl_err);
+}
+
+static void nfnl_err_deliver(struct list_head *err_list, struct sk_buff *skb)
+{
+	struct nfnl_err *nfnl_err, *next;
+
+	list_for_each_entry_safe(nfnl_err, next, err_list, head) {
+		netlink_ack(skb, nfnl_err->nlh, nfnl_err->err);
+		nfnl_err_del(nfnl_err);
+	}
+}
+
 static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
 				u_int16_t subsys_id)
 {
@@ -230,6 +275,7 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
 	const struct nfnetlink_subsystem *ss;
 	const struct nfnl_callback *nc;
 	bool success = true, done = false;
+	static LIST_HEAD(err_list);
 	int err;
 
 	if (subsys_id >= NFNL_SUBSYS_COUNT)
@@ -287,6 +333,7 @@ replay:
 		type = nlh->nlmsg_type;
 		if (type == NFNL_MSG_BATCH_BEGIN) {
 			/* Malformed: Batch begin twice */
+			nfnl_err_reset(&err_list);
 			success = false;
 			goto done;
 		} else if (type == NFNL_MSG_BATCH_END) {
@@ -333,6 +380,7 @@ replay:
 			 * original skb.
 			 */
 			if (err == -EAGAIN) {
+				nfnl_err_reset(&err_list);
 				ss->abort(skb);
 				nfnl_unlock(subsys_id);
 				kfree_skb(nskb);
@@ -341,11 +389,24 @@ replay:
 		}
 ack:
 		if (nlh->nlmsg_flags & NLM_F_ACK || err) {
+			/* Errors are delivered once the full batch has been
+			 * processed, this avoids that the same error is
+			 * reported several times when replaying the batch.
+			 */
+			if (nfnl_err_add(&err_list, nlh, err) < 0) {
+				/* We failed to enqueue an error, reset the
+				 * list of errors and send OOM to userspace
+				 * pointing to the batch header.
+				 */
+				nfnl_err_reset(&err_list);
+				netlink_ack(skb, nlmsg_hdr(oskb), -ENOMEM);
+				success = false;
+				goto done;
+			}
 			/* We don't stop processing the batch on errors, thus,
 			 * userspace gets all the errors that the batch
 			 * triggers.
 			 */
-			netlink_ack(skb, nlh, err);
 			if (err)
 				success = false;
 		}
@@ -361,6 +422,7 @@ done:
 	else
 		ss->abort(skb);
 
+	nfnl_err_deliver(&err_list, oskb);
 	nfnl_unlock(subsys_id);
 	kfree_skb(nskb);
 }
-- 
1.7.10.4

  parent reply	other threads:[~2014-09-23  9:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-23  9:24 [PATCH 0/5] nf pull request for net Pablo Neira Ayuso
2014-09-23  9:24 ` [PATCH 1/5] netfilter: nft_hash: no need for rcu in the hash set destroy path Pablo Neira Ayuso
2014-09-23  9:24 ` [PATCH 2/5] netfilter: nft_rbtree: no need for spinlock from " Pablo Neira Ayuso
2014-09-23  9:52   ` Eric Dumazet
2014-09-23 11:01     ` Pablo Neira Ayuso
2014-09-23 11:54       ` Eric Dumazet
2014-09-23 16:10         ` Pablo Neira Ayuso
2014-09-23 16:29           ` Eric Dumazet
2014-09-23  9:24 ` [PATCH 3/5] rhashtable: fix lockdep splat in rhashtable_destroy() Pablo Neira Ayuso
2014-09-23  9:24 ` Pablo Neira Ayuso [this message]
2014-09-23  9:24 ` [PATCH 5/5] netfilter: xt_TPROXY: undefined reference to `udp6_lib_lookup' Pablo Neira Ayuso
2014-09-26 20:21 ` [PATCH 0/5] nf pull request for net David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1411464288-18069-5-git-send-email-pablo@netfilter.org \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).