All of lore.kernel.org
 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 06/13] netfilter: nf_tables: fix base chain stat rcu_dereference usage
Date: Mon, 13 May 2019 11:56:23 +0200	[thread overview]
Message-ID: <20190513095630.32443-7-pablo@netfilter.org> (raw)
In-Reply-To: <20190513095630.32443-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Following splat gets triggered when nfnetlink monitor is running while
xtables-nft selftests are running:

net/netfilter/nf_tables_api.c:1272 suspicious rcu_dereference_check() usage!
other info that might help us debug this:

1 lock held by xtables-nft-mul/27006:
 #0: 00000000e0f85be9 (&net->nft.commit_mutex){+.+.}, at: nf_tables_valid_genid+0x1a/0x50
Call Trace:
 nf_tables_fill_chain_info.isra.45+0x6cc/0x6e0
 nf_tables_chain_notify+0xf8/0x1a0
 nf_tables_commit+0x165c/0x1740

nf_tables_fill_chain_info() can be called both from dumps (rcu read locked)
or from the transaction path if a userspace process subscribed to nftables
notifications.

In the 'table dump' case, rcu_access_pointer() cannot be used: We do not
hold transaction mutex so the pointer can be NULLed right after the check.
Just unconditionally fetch the value, then have the helper return
immediately if its NULL.

In the notification case we don't hold the rcu read lock, but updates are
prevented due to transaction mutex. Use rcu_dereference_check() to make lockdep
aware of this.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index f2e12ae50544..e4f6ecac48c3 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1190,6 +1190,9 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
 	u64 pkts, bytes;
 	int cpu;
 
+	if (!stats)
+		return 0;
+
 	memset(&total, 0, sizeof(total));
 	for_each_possible_cpu(cpu) {
 		cpu_stats = per_cpu_ptr(stats, cpu);
@@ -1247,6 +1250,7 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
 	if (nft_is_base_chain(chain)) {
 		const struct nft_base_chain *basechain = nft_base_chain(chain);
 		const struct nf_hook_ops *ops = &basechain->ops;
+		struct nft_stats __percpu *stats;
 		struct nlattr *nest;
 
 		nest = nla_nest_start(skb, NFTA_CHAIN_HOOK);
@@ -1268,8 +1272,9 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
 		if (nla_put_string(skb, NFTA_CHAIN_TYPE, basechain->type->name))
 			goto nla_put_failure;
 
-		if (rcu_access_pointer(basechain->stats) &&
-		    nft_dump_stats(skb, rcu_dereference(basechain->stats)))
+		stats = rcu_dereference_check(basechain->stats,
+					      lockdep_commit_lock_is_held(net));
+		if (nft_dump_stats(skb, stats))
 			goto nla_put_failure;
 	}
 
-- 
2.11.0


  parent reply	other threads:[~2019-05-13  9:56 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-13  9:56 [PATCH 00/13] Netfilter fixes for net Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 01/13] netfilter: nf_tables: delay chain policy update until transaction is complete Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 02/13] netfilter: nft_flow_offload: add entry to flowtable after confirmation Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 03/13] netfilter: nf_flow_table: fix netdev refcnt leak Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 04/13] netfilter: nf_flow_table: check ttl value in flow offload data path Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 05/13] netfilter: nf_conntrack_h323: restore boundary check correctness Pablo Neira Ayuso
2019-05-13  9:56 ` Pablo Neira Ayuso [this message]
2019-05-13  9:56 ` [PATCH 07/13] netfilter: nf_flow_table: fix missing error check for rhashtable_insert_fast Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 08/13] netfilter: ctnetlink: Resolve conntrack L3-protocol flush regression Pablo Neira Ayuso
2019-06-24  3:44   ` Felix Kaechele
2019-06-24 23:58     ` Pablo Neira Ayuso
2019-06-25  3:02       ` Felix Kaechele
2019-06-25  8:08         ` Pablo Neira Ayuso
2019-06-25 11:33           ` Felix Kaechele
2019-06-25 11:52             ` Kristian Evensen
2019-06-25 14:45               ` Felix Kaechele
2019-06-25 15:08                 ` Kristian Evensen
2019-06-25 16:16                   ` Felix Kaechele
2019-06-25 19:45                     ` Pablo Neira Ayuso
2019-06-25 15:45                 ` Kristian Evensen
2019-06-25 19:40               ` Pablo Neira Ayuso
2019-06-25 19:41             ` Pablo Neira Ayuso
2019-06-25 15:01       ` Nicolas Dichtel
2019-06-25 19:44         ` Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 09/13] netfilter: nf_conntrack_h323: Remove deprecated config check Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 10/13] netfilter: nf_flow_table: do not flow offload deleted conntrack entries Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 11/13] netfilter: ebtables: CONFIG_COMPAT: reject trailing data after last rule Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 12/13] netfilter: nf_tables: remove NFT_CT_TIMEOUT Pablo Neira Ayuso
2019-05-13  9:56 ` [PATCH 13/13] netfilter: nf_tables: correct NFT_LOGLEVEL_MAX value Pablo Neira Ayuso
2019-05-13 16:02 ` [PATCH 00/13] 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=20190513095630.32443-7-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.