[PATCH nft 2/2] netfilter: nft_compat: don't use refcount_inc on newly allocated entry

[Florian Westphal]

When I moved the refcount to refcount_t type I missed the fact that
refcount_inc() will result in use-after-free warning with
CONFIG_REFCOUNT_FULL=y builds.

The correct fix would be to init the reference count to 1 at allocation
time, but, unfortunately we cannot do this, as we can't undo that
in case something else fails later in the batch.

So only solution I see is to special-case the 'new entry' condition
and replace refcount_inc() with a "delayed" refcount_set(1) in this case.

Fixes: 12c44aba6618 ("netfilter: nft_compat: use refcnt_t type for nft_xt 
reference count")
Reported-by: Jordan Glover <golden_mille...@protonmail.ch>
Signed-off-by: Florian Westphal <f...@strlen.de>
---
 net/netfilter/nft_compat.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 13c5ecc4d7b2..fe64df848365 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -61,6 +61,21 @@ static struct nft_compat_net *nft_compat_pernet(struct net 
*net)
        return net_generic(net, nft_compat_net_id);
 }
 
+static void nft_xt_get(struct nft_xt *xt)
+{
+       /* refcount_inc() warns on 0 -> 1 transition, but we can't
+        * init the reference count to 1 in .select_ops -- we can't
+        * undo such an increase when another expression inside the same
+        * rule fails afterwards.
+        */
+       if (xt->listcnt == 0)
+               refcount_set(&xt->refcnt, 1);
+       else
+               refcount_inc(&xt->refcnt);
+
+       xt->listcnt++;
+}
+
 static bool nft_xt_put(struct nft_xt *xt)
 {
        if (refcount_dec_and_test(&xt->refcnt)) {
@@ -291,7 +306,7 @@ nft_target_init(const struct nft_ctx *ctx, const struct 
nft_expr *expr,
                return -EINVAL;
 
        nft_xt = container_of(expr->ops, struct nft_xt, ops);
-       refcount_inc(&nft_xt->refcnt);
+       nft_xt_get(nft_xt);
        return 0;
 }
 
@@ -504,7 +519,7 @@ __nft_match_init(const struct nft_ctx *ctx, const struct 
nft_expr *expr,
                return ret;
 
        nft_xt = container_of(expr->ops, struct nft_xt, ops);
-       refcount_inc(&nft_xt->refcnt);
+       nft_xt_get(nft_xt);
        return 0;
 }
 
@@ -840,7 +855,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
 
        nft_match->ops.size = matchsize;
 
-       nft_match->listcnt = 1;
+       nft_match->listcnt = 0;
        list_add(&nft_match->head, &cn->nft_match_list);
 
        return &nft_match->ops;
@@ -937,7 +952,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
        else
                nft_target->ops.eval = nft_target_eval_xt;
 
-       nft_target->listcnt = 1;
+       nft_target->listcnt = 0;
        list_add(&nft_target->head, &cn->nft_target_list);
 
        return &nft_target->ops;
-- 
2.19.2