]>
Commit | Line | Data |
---|---|---|
37554d48 SL |
1 | From 9f087a4c3ce788d8ed718fb8cbc2b169c944f30f Mon Sep 17 00:00:00 2001 |
2 | From: Florian Westphal <fw@strlen.de> | |
3 | Date: Tue, 30 Apr 2019 14:33:22 +0200 | |
4 | Subject: netfilter: nf_tables: fix base chain stat rcu_dereference usage | |
5 | ||
6 | [ Upstream commit edbd82c5fba009f68d20b5db585be1e667c605f6 ] | |
7 | ||
8 | Following splat gets triggered when nfnetlink monitor is running while | |
9 | xtables-nft selftests are running: | |
10 | ||
11 | net/netfilter/nf_tables_api.c:1272 suspicious rcu_dereference_check() usage! | |
12 | other info that might help us debug this: | |
13 | ||
14 | 1 lock held by xtables-nft-mul/27006: | |
15 | #0: 00000000e0f85be9 (&net->nft.commit_mutex){+.+.}, at: nf_tables_valid_genid+0x1a/0x50 | |
16 | Call Trace: | |
17 | nf_tables_fill_chain_info.isra.45+0x6cc/0x6e0 | |
18 | nf_tables_chain_notify+0xf8/0x1a0 | |
19 | nf_tables_commit+0x165c/0x1740 | |
20 | ||
21 | nf_tables_fill_chain_info() can be called both from dumps (rcu read locked) | |
22 | or from the transaction path if a userspace process subscribed to nftables | |
23 | notifications. | |
24 | ||
25 | In the 'table dump' case, rcu_access_pointer() cannot be used: We do not | |
26 | hold transaction mutex so the pointer can be NULLed right after the check. | |
27 | Just unconditionally fetch the value, then have the helper return | |
28 | immediately if its NULL. | |
29 | ||
30 | In the notification case we don't hold the rcu read lock, but updates are | |
31 | prevented due to transaction mutex. Use rcu_dereference_check() to make lockdep | |
32 | aware of this. | |
33 | ||
34 | Signed-off-by: Florian Westphal <fw@strlen.de> | |
35 | Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> | |
36 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
37 | --- | |
38 | net/netfilter/nf_tables_api.c | 9 +++++++-- | |
39 | 1 file changed, 7 insertions(+), 2 deletions(-) | |
40 | ||
41 | diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c | |
42 | index ebfcfe1dcbdb..29ff59dd99ac 100644 | |
43 | --- a/net/netfilter/nf_tables_api.c | |
44 | +++ b/net/netfilter/nf_tables_api.c | |
45 | @@ -1142,6 +1142,9 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats) | |
46 | u64 pkts, bytes; | |
47 | int cpu; | |
48 | ||
49 | + if (!stats) | |
50 | + return 0; | |
51 | + | |
52 | memset(&total, 0, sizeof(total)); | |
53 | for_each_possible_cpu(cpu) { | |
54 | cpu_stats = per_cpu_ptr(stats, cpu); | |
55 | @@ -1199,6 +1202,7 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net, | |
56 | if (nft_is_base_chain(chain)) { | |
57 | const struct nft_base_chain *basechain = nft_base_chain(chain); | |
58 | const struct nf_hook_ops *ops = &basechain->ops; | |
59 | + struct nft_stats __percpu *stats; | |
60 | struct nlattr *nest; | |
61 | ||
62 | nest = nla_nest_start(skb, NFTA_CHAIN_HOOK); | |
63 | @@ -1220,8 +1224,9 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net, | |
64 | if (nla_put_string(skb, NFTA_CHAIN_TYPE, basechain->type->name)) | |
65 | goto nla_put_failure; | |
66 | ||
67 | - if (rcu_access_pointer(basechain->stats) && | |
68 | - nft_dump_stats(skb, rcu_dereference(basechain->stats))) | |
69 | + stats = rcu_dereference_check(basechain->stats, | |
70 | + lockdep_commit_lock_is_held(net)); | |
71 | + if (nft_dump_stats(skb, stats)) | |
72 | goto nla_put_failure; | |
73 | } | |
74 | ||
75 | -- | |
76 | 2.20.1 | |
77 |