netdev.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 10/11] netfilter: nf_tables: fix infinite loop when expr is not available
Date: Fri,  6 Mar 2020 19:15:12 +0100	[thread overview]
Message-ID: <20200306181513.656594-11-pablo@netfilter.org> (raw)
In-Reply-To: <20200306181513.656594-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

nft will loop forever if the kernel doesn't support an expression:

1. nft_expr_type_get() appends the family specific name to the module list.
2. -EAGAIN is returned to nfnetlink, nfnetlink calls abort path.
3. abort path sets ->done to true and calls request_module for the
   expression.
4. nfnetlink replays the batch, we end up in nft_expr_type_get() again.
5. nft_expr_type_get attempts to append family-specific name. This
   one already exists on the list, so we continue
6. nft_expr_type_get adds the generic expression name to the module
   list. -EAGAIN is returned, nfnetlink calls abort path.
7. abort path encounters the family-specific expression which
   has 'done' set, so it gets removed.
8. abort path requests the generic expression name, sets done to true.
9. batch is replayed.

If the expression could not be loaded, then we will end up back at 1),
because the family-specific name got removed and the cycle starts again.

Note that userspace can SIGKILL the nft process to stop the cycle, but
the desired behaviour is to return an error after the generic expr name
fails to load the expression.

Fixes: eb014de4fd418 ("netfilter: nf_tables: autoload modules from the abort path")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index f9e60981bd36..38c680f28f15 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -7388,13 +7388,8 @@ static void nf_tables_module_autoload(struct net *net)
 	list_splice_init(&net->nft.module_list, &module_list);
 	mutex_unlock(&net->nft.commit_mutex);
 	list_for_each_entry_safe(req, next, &module_list, list) {
-		if (req->done) {
-			list_del(&req->list);
-			kfree(req);
-		} else {
-			request_module("%s", req->module);
-			req->done = true;
-		}
+		request_module("%s", req->module);
+		req->done = true;
 	}
 	mutex_lock(&net->nft.commit_mutex);
 	list_splice(&module_list, &net->nft.module_list);
@@ -8177,6 +8172,7 @@ static void __net_exit nf_tables_exit_net(struct net *net)
 	__nft_release_tables(net);
 	mutex_unlock(&net->nft.commit_mutex);
 	WARN_ON_ONCE(!list_empty(&net->nft.tables));
+	WARN_ON_ONCE(!list_empty(&net->nft.module_list));
 }
 
 static struct pernet_operations nf_tables_net_ops = {
-- 
2.11.0


  parent reply	other threads:[~2020-03-06 18:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06 18:15 [PATCH 00/11] Netfilter fixes for net Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 01/11] netfilter: nf_conntrack: ct_cpu_seq_next should increase position index Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 02/11] netfilter: synproxy: synproxy_cpu_seq_next " Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 03/11] netfilter: xt_recent: recent_seq_next " Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 04/11] netfilter: x_tables: xt_mttg_seq_next " Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 05/11] netfilter: nf_tables: free flowtable hooks on hook register error Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 06/11] netfilter: cthelper: add missing attribute validation for cthelper Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 07/11] netfilter: nft_payload: add missing attribute validation for payload csum flags Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 08/11] netfilter: nft_tunnel: add missing attribute validation for tunnels Pablo Neira Ayuso
2020-03-06 18:15 ` [PATCH 09/11] netfilter: nf_tables: dump NFTA_CHAIN_FLAGS attribute Pablo Neira Ayuso
2020-03-06 18:15 ` Pablo Neira Ayuso [this message]
2020-03-06 18:15 ` [PATCH 11/11] netfilter: nft_chain_nat: inet family is missing module ownership Pablo Neira Ayuso
2020-03-07  5:38 ` [PATCH 00/11] Netfilter fixes 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=20200306181513.656594-11-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).