Message ID | 1654503264-47182-1-git-send-email-akaher@vmware.com |
---|---|
State | Awaiting Upstream |
Delegated to: | Pablo Neira |
Headers | show |
Series | [v5.4.y] netfilter: nf_tables: disallow non-stateful expression in sets earlier | expand |
On Mon, Jun 06, 2022 at 01:44:24PM +0530, Ajay Kaher wrote: > From: Pablo Neira Ayuso <pablo@netfilter.org> > > commit 520778042ccca019f3ffa136dd0ca565c486cedd upstream. > > Since 3e135cd499bf ("netfilter: nft_dynset: dynamic stateful expression > instantiation"), it is possible to attach stateful expressions to set > elements. > > cd5125d8f518 ("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: 0b2d8a7b638b ("netfilter: nf_tables: add helper functions for expression handling") > Reported-and-tested-by: Aaron Adams <edg-e@nccgroup.com> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > [Ajay: Regenerated the patch for v5.4.y] > Signed-off-by: Ajay Kaher <akaher@vmware.com> > --- > net/netfilter/nf_tables_api.c | 16 ++++++++++------ > net/netfilter/nft_dynset.c | 3 --- > 2 files changed, 10 insertions(+), 9 deletions(-) Both backports now queued up, thanks. greg k-h
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 545da27..b51c192 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -2267,27 +2267,31 @@ struct nft_expr *nft_expr_init(const struct nft_ctx *ctx, err = nf_tables_expr_parse(ctx, nla, &info); if (err < 0) - goto err1; + goto err_expr_parse; + + err = -EOPNOTSUPP; + if (!(info.ops->type->flags & NFT_EXPR_STATEFUL)) + goto err_expr_stateful; err = -ENOMEM; expr = kzalloc(info.ops->size, GFP_KERNEL); if (expr == NULL) - goto err2; + goto err_expr_stateful; err = nf_tables_newexpr(ctx, &info, expr); if (err < 0) - goto err3; + goto err_expr_new; return expr; -err3: +err_expr_new: kfree(expr); -err2: +err_expr_stateful: owner = info.ops->type->owner; if (info.ops->type->release_ops) info.ops->type->release_ops(info.ops); module_put(owner); -err1: +err_expr_parse: return ERR_PTR(err); } diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c index 6fdea0e..6bcc181 100644 --- a/net/netfilter/nft_dynset.c +++ b/net/netfilter/nft_dynset.c @@ -204,9 +204,6 @@ static int nft_dynset_init(const struct nft_ctx *ctx, return PTR_ERR(priv->expr); err = -EOPNOTSUPP; - if (!(priv->expr->ops->type->flags & NFT_EXPR_STATEFUL)) - goto err1; - if (priv->expr->ops->type->flags & NFT_EXPR_GC) { if (set->flags & NFT_SET_TIMEOUT) goto err1;