queue-5.0/netfilter-nf_tables-fix-set-double-free-in-abort-pat.patch

   1 From 3152556eacc9d0643cb244c5aecdda70530ded63 Mon Sep 17 00:00:00 2001
   2 From: Pablo Neira Ayuso <pablo@netfilter.org>
   3 Date: Fri, 8 Mar 2019 00:58:53 +0100
   4 Subject: netfilter: nf_tables: fix set double-free in abort path
   5
   6 [ Upstream commit 40ba1d9b4d19796afc9b7ece872f5f3e8f5e2c13 ]
   7
   8 The abort path can cause a double-free of an anonymous set.
   9 Added-and-to-be-aborted rule looks like this:
  10
  11 udp dport { 137, 138 } drop
  12
  13 The to-be-aborted transaction list looks like this:
  14
  15 newset
  16 newsetelem
  17 newsetelem
  18 rule
  19
  20 This gets walked in reverse order, so first pass disables the rule, the
  21 set elements, then the set.
  22
  23 After synchronize_rcu(), we then destroy those in same order: rule, set
  24 element, set element, newset.
  25
  26 Problem is that the anonymous set has already been bound to the rule, so
  27 the rule (lookup expression destructor) already frees the set, when then
  28 cause use-after-free when trying to delete the elements from this set,
  29 then try to free the set again when handling the newset expression.
  30
  31 Rule releases the bound set in first place from the abort path, this
  32 causes the use-after-free on set element removal when undoing the new
  33 element transactions. To handle this, skip new element transaction if
  34 set is bound from the abort path.
  35
  36 This is still causes the use-after-free on set element removal.  To
  37 handle this, remove transaction from the list when the set is already
  38 bound.
  39
  40 Joint work with Florian Westphal.
  41
  42 Fixes: f6ac85858976 ("netfilter: nf_tables: unbind set in rule from commit path")
  43 Bugzilla: https://bugzilla.netfilter.org/show_bug.cgi?id=1325
  44 Acked-by: Florian Westphal <fw@strlen.de>
  45 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  46 Signed-off-by: Sasha Levin <sashal@kernel.org>
  47 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  48 ---
  49  include/net/netfilter/nf_tables.h |    6 ++----
  50  net/netfilter/nf_tables_api.c     |   17 +++++++++++------
  51  2 files changed, 13 insertions(+), 10 deletions(-)
  52
  53 --- a/include/net/netfilter/nf_tables.h
  54 +++ b/include/net/netfilter/nf_tables.h
  55 @@ -416,7 +416,8 @@ struct nft_set {
  56         unsigned char                   *udata;
  57         /* runtime data below here */
  58         const struct nft_set_ops        *ops ____cacheline_aligned;
  59 -       u16                             flags:14,
  60 +       u16                             flags:13,
  61 +                                       bound:1,
  62                                         genmask:2;
  63         u8                              klen;
  64         u8                              dlen;
  65 @@ -1329,15 +1330,12 @@ struct nft_trans_rule {
  66  struct nft_trans_set {
  67         struct nft_set                  *set;
  68         u32                             set_id;
  69 -       bool                            bound;
  70  };
  71
  72  #define nft_trans_set(trans)   \
  73         (((struct nft_trans_set *)trans->data)->set)
  74  #define nft_trans_set_id(trans)        \
  75         (((struct nft_trans_set *)trans->data)->set_id)
  76 -#define nft_trans_set_bound(trans)     \
  77 -       (((struct nft_trans_set *)trans->data)->bound)
  78
  79  struct nft_trans_chain {
  80         bool                            update;
  81 --- a/net/netfilter/nf_tables_api.c
  82 +++ b/net/netfilter/nf_tables_api.c
  83 @@ -127,7 +127,7 @@ static void nft_set_trans_bind(const str
  84         list_for_each_entry_reverse(trans, &net->nft.commit_list, list) {
  85                 if (trans->msg_type == NFT_MSG_NEWSET &&
  86                     nft_trans_set(trans) == set) {
  87 -                       nft_trans_set_bound(trans) = true;
  88 +                       set->bound = true;
  89                         break;
  90                 }
  91         }
  92 @@ -6617,8 +6617,7 @@ static void nf_tables_abort_release(stru
  93                 nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans));
  94                 break;
  95         case NFT_MSG_NEWSET:
  96 -               if (!nft_trans_set_bound(trans))
  97 -                       nft_set_destroy(nft_trans_set(trans));
  98 +               nft_set_destroy(nft_trans_set(trans));
  99                 break;
 100         case NFT_MSG_NEWSETELEM:
 101                 nft_set_elem_destroy(nft_trans_elem_set(trans),
 102 @@ -6691,8 +6690,11 @@ static int __nf_tables_abort(struct net
 103                         break;
 104                 case NFT_MSG_NEWSET:
 105                         trans->ctx.table->use--;
 106 -                       if (!nft_trans_set_bound(trans))
 107 -                               list_del_rcu(&nft_trans_set(trans)->list);
 108 +                       if (nft_trans_set(trans)->bound) {
 109 +                               nft_trans_destroy(trans);
 110 +                               break;
 111 +                       }
 112 +                       list_del_rcu(&nft_trans_set(trans)->list);
 113                         break;
 114                 case NFT_MSG_DELSET:
 115                         trans->ctx.table->use++;
 116 @@ -6700,8 +6702,11 @@ static int __nf_tables_abort(struct net
 117                         nft_trans_destroy(trans);
 118                         break;
 119                 case NFT_MSG_NEWSETELEM:
 120 +                       if (nft_trans_elem_set(trans)->bound) {
 121 +                               nft_trans_destroy(trans);
 122 +                               break;
 123 +                       }
 124                         te = (struct nft_trans_elem *)trans->data;
 125 -
 126                         te->set->ops->remove(net, te->set, &te->elem);
 127                         atomic_dec(&te->set->nelems);
 128                         break;