queue-4.20/netfilter-nft_compat-don-t-use-refcount_inc-on-newly.patch

   1 From 998c1eb43f8ea522a08fea1e866dfa68208c5a7c Mon Sep 17 00:00:00 2001
   2 From: Florian Westphal <fw@strlen.de>
   3 Date: Tue, 5 Feb 2019 12:16:18 +0100
   4 Subject: netfilter: nft_compat: don't use refcount_inc on newly allocated
   5  entry
   6
   7 [ Upstream commit 947e492c0fc2132ae5fca081a9c2952ccaab0404 ]
   8
   9 When I moved the refcount to refcount_t type I missed the fact that
  10 refcount_inc() will result in use-after-free warning with
  11 CONFIG_REFCOUNT_FULL=y builds.
  12
  13 The correct fix would be to init the reference count to 1 at allocation
  14 time, but, unfortunately we cannot do this, as we can't undo that
  15 in case something else fails later in the batch.
  16
  17 So only solution I see is to special-case the 'new entry' condition
  18 and replace refcount_inc() with a "delayed" refcount_set(1) in this case,
  19 as done here.
  20
  21 The .activate callback can be removed to simplify things, we only
  22 need to make sure that deactivate() decrements/unlinks the entry
  23 from the list at end of transaction phase (commit or abort).
  24
  25 Fixes: 12c44aba6618 ("netfilter: nft_compat: use refcnt_t type for nft_xt reference count")
  26 Reported-by: Jordan Glover <Golden_Miller83@protonmail.ch>
  27 Signed-off-by: Florian Westphal <fw@strlen.de>
  28 Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  29 Signed-off-by: Sasha Levin <sashal@kernel.org>
  30 ---
  31  net/netfilter/nft_compat.c | 62 ++++++++++++++------------------------
  32  1 file changed, 23 insertions(+), 39 deletions(-)
  33
  34 diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
  35 index 5fd94f7fdb94..0a4bad55a8aa 100644
  36 --- a/net/netfilter/nft_compat.c
  37 +++ b/net/netfilter/nft_compat.c
  38 @@ -61,6 +61,21 @@ static struct nft_compat_net *nft_compat_pernet(struct net *net)
  39         return net_generic(net, nft_compat_net_id);
  40  }
  41
  42 +static void nft_xt_get(struct nft_xt *xt)
  43 +{
  44 +       /* refcount_inc() warns on 0 -> 1 transition, but we can't
  45 +        * init the reference count to 1 in .select_ops -- we can't
  46 +        * undo such an increase when another expression inside the same
  47 +        * rule fails afterwards.
  48 +        */
  49 +       if (xt->listcnt == 0)
  50 +               refcount_set(&xt->refcnt, 1);
  51 +       else
  52 +               refcount_inc(&xt->refcnt);
  53 +
  54 +       xt->listcnt++;
  55 +}
  56 +
  57  static bool nft_xt_put(struct nft_xt *xt)
  58  {
  59         if (refcount_dec_and_test(&xt->refcnt)) {
  60 @@ -291,7 +306,7 @@ nft_target_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
  61                 return -EINVAL;
  62
  63         nft_xt = container_of(expr->ops, struct nft_xt, ops);
  64 -       refcount_inc(&nft_xt->refcnt);
  65 +       nft_xt_get(nft_xt);
  66         return 0;
  67  }
  68
  69 @@ -505,7 +520,7 @@ __nft_match_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
  70                 return ret;
  71
  72         nft_xt = container_of(expr->ops, struct nft_xt, ops);
  73 -       refcount_inc(&nft_xt->refcnt);
  74 +       nft_xt_get(nft_xt);
  75         return 0;
  76  }
  77
  78 @@ -559,45 +574,16 @@ nft_match_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr)
  79         __nft_match_destroy(ctx, expr, nft_expr_priv(expr));
  80  }
  81
  82 -static void nft_compat_activate(const struct nft_ctx *ctx,
  83 -                               const struct nft_expr *expr,
  84 -                               struct list_head *h)
  85 -{
  86 -       struct nft_xt *xt = container_of(expr->ops, struct nft_xt, ops);
  87 -
  88 -       if (xt->listcnt == 0)
  89 -               list_add(&xt->head, h);
  90 -
  91 -       xt->listcnt++;
  92 -}
  93 -
  94 -static void nft_compat_activate_mt(const struct nft_ctx *ctx,
  95 -                                  const struct nft_expr *expr)
  96 -{
  97 -       struct nft_compat_net *cn = nft_compat_pernet(ctx->net);
  98 -
  99 -       nft_compat_activate(ctx, expr, &cn->nft_match_list);
 100 -}
 101 -
 102 -static void nft_compat_activate_tg(const struct nft_ctx *ctx,
 103 -                                  const struct nft_expr *expr)
 104 -{
 105 -       struct nft_compat_net *cn = nft_compat_pernet(ctx->net);
 106 -
 107 -       nft_compat_activate(ctx, expr, &cn->nft_target_list);
 108 -}
 109 -
 110  static void nft_compat_deactivate(const struct nft_ctx *ctx,
 111                                   const struct nft_expr *expr,
 112                                   enum nft_trans_phase phase)
 113  {
 114         struct nft_xt *xt = container_of(expr->ops, struct nft_xt, ops);
 115
 116 -       if (phase == NFT_TRANS_COMMIT)
 117 -               return;
 118 -
 119 -       if (--xt->listcnt == 0)
 120 -               list_del_init(&xt->head);
 121 +       if (phase == NFT_TRANS_ABORT || phase == NFT_TRANS_COMMIT) {
 122 +               if (--xt->listcnt == 0)
 123 +                       list_del_init(&xt->head);
 124 +       }
 125  }
 126
 127  static void
 128 @@ -853,7 +839,6 @@ nft_match_select_ops(const struct nft_ctx *ctx,
 129         nft_match->ops.eval = nft_match_eval;
 130         nft_match->ops.init = nft_match_init;
 131         nft_match->ops.destroy = nft_match_destroy;
 132 -       nft_match->ops.activate = nft_compat_activate_mt;
 133         nft_match->ops.deactivate = nft_compat_deactivate;
 134         nft_match->ops.dump = nft_match_dump;
 135         nft_match->ops.validate = nft_match_validate;
 136 @@ -871,7 +856,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
 137
 138         nft_match->ops.size = matchsize;
 139
 140 -       nft_match->listcnt = 1;
 141 +       nft_match->listcnt = 0;
 142         list_add(&nft_match->head, &cn->nft_match_list);
 143
 144         return &nft_match->ops;
 145 @@ -958,7 +943,6 @@ nft_target_select_ops(const struct nft_ctx *ctx,
 146         nft_target->ops.size = NFT_EXPR_SIZE(XT_ALIGN(target->targetsize));
 147         nft_target->ops.init = nft_target_init;
 148         nft_target->ops.destroy = nft_target_destroy;
 149 -       nft_target->ops.activate = nft_compat_activate_tg;
 150         nft_target->ops.deactivate = nft_compat_deactivate;
 151         nft_target->ops.dump = nft_target_dump;
 152         nft_target->ops.validate = nft_target_validate;
 153 @@ -969,7 +953,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
 154         else
 155                 nft_target->ops.eval = nft_target_eval_xt;
 156
 157 -       nft_target->listcnt = 1;
 158 +       nft_target->listcnt = 0;
 159         list_add(&nft_target->head, &cn->nft_target_list);
 160
 161         return &nft_target->ops;
 162 --
 163 2.19.1
 164