Skip to content

Commit f692bcf

Browse files
ummakynesgregkh
authored andcommitted
netfilter: nf_tables: disallow non-stateful expression in sets earlier
commit 5207780 upstream. Since 3e135cd ("netfilter: nft_dynset: dynamic stateful expression instantiation"), it is possible to attach stateful expressions to set elements. cd5125d ("netfilter: nf_tables: split set destruction in deactivate and destroy phase") introduces conditional destruction on the object to accomodate transaction semantics. nft_expr_init() calls expr->ops->init() first, then check for NFT_STATEFUL_EXPR, this stills allows to initialize a non-stateful lookup expressions which points to a set, which might lead to UAF since the set is not properly detached from the set->binding for this case. Anyway, this combination is non-sense from nf_tables perspective. This patch fixes this problem by checking for NFT_STATEFUL_EXPR before expr->ops->init() is called. The reporter provides a KASAN splat and a poc reproducer (similar to those autogenerated by syzbot to report use-after-free errors). It is unknown to me if they are using syzbot or if they use similar automated tool to locate the bug that they are reporting. For the record, this is the KASAN splat. [ 85.431824] ================================================================== [ 85.432901] BUG: KASAN: use-after-free in nf_tables_bind_set+0x81b/0xa20 [ 85.433825] Write of size 8 at addr ffff8880286f0e98 by task poc/776 [ 85.434756] [ 85.434999] CPU: 1 PID: 776 Comm: poc Tainted: G W 5.18.0+ #2 [ 85.436023] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014 Fixes: 0b2d8a7 ("netfilter: nf_tables: add helper functions for expression handling") Reported-and-tested-by: Aaron Adams <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent f55c75c commit f692bcf

File tree

1 file changed

+10
-9
lines changed

1 file changed

+10
-9
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2778,27 +2778,31 @@ static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
27782778

27792779
err = nf_tables_expr_parse(ctx, nla, &expr_info);
27802780
if (err < 0)
2781-
goto err1;
2781+
goto err_expr_parse;
2782+
2783+
err = -EOPNOTSUPP;
2784+
if (!(expr_info.ops->type->flags & NFT_EXPR_STATEFUL))
2785+
goto err_expr_stateful;
27822786

27832787
err = -ENOMEM;
27842788
expr = kzalloc(expr_info.ops->size, GFP_KERNEL);
27852789
if (expr == NULL)
2786-
goto err2;
2790+
goto err_expr_stateful;
27872791

27882792
err = nf_tables_newexpr(ctx, &expr_info, expr);
27892793
if (err < 0)
2790-
goto err3;
2794+
goto err_expr_new;
27912795

27922796
return expr;
2793-
err3:
2797+
err_expr_new:
27942798
kfree(expr);
2795-
err2:
2799+
err_expr_stateful:
27962800
owner = expr_info.ops->type->owner;
27972801
if (expr_info.ops->type->release_ops)
27982802
expr_info.ops->type->release_ops(expr_info.ops);
27992803

28002804
module_put(owner);
2801-
err1:
2805+
err_expr_parse:
28022806
return ERR_PTR(err);
28032807
}
28042808

@@ -5318,9 +5322,6 @@ struct nft_expr *nft_set_elem_expr_alloc(const struct nft_ctx *ctx,
53185322
return expr;
53195323

53205324
err = -EOPNOTSUPP;
5321-
if (!(expr->ops->type->flags & NFT_EXPR_STATEFUL))
5322-
goto err_set_elem_expr;
5323-
53245325
if (expr->ops->type->flags & NFT_EXPR_GC) {
53255326
if (set->flags & NFT_SET_TIMEOUT)
53265327
goto err_set_elem_expr;

0 commit comments

Comments
 (0)