queue-4.19/netfilter-nf_tables-fix-base-chain-stat-rcu_derefere.patch

   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