Hi Greg, I'll take care of this. Thanks. On Mon, Jun 19, 2023 at 09:43:33AM +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote: > > The patch below does not apply to the 5.10-stable tree. > If someone wants it applied there, or to any other stable or longterm > tree, then please email the backport, including the original git commit > id to <stable@xxxxxxxxxxxxxxx>. > > To reproduce the conflict and resubmit, you may use the following commands: > > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y > git checkout FETCH_HEAD > git cherry-pick -x 1240eb93f0616b21c675416516ff3d74798fdc97 > # <resolve conflicts, build, test, etc.> > git commit -s > git send-email --to '<stable@xxxxxxxxxxxxxxx>' --in-reply-to '2023061933-remover-tweet-3f9b@gregkh' --subject-prefix 'PATCH 5.10.y' HEAD^.. > > Possible dependencies: > > > > thanks, > > greg k-h > > ------------------ original commit in Linus's tree ------------------ > > From 1240eb93f0616b21c675416516ff3d74798fdc97 Mon Sep 17 00:00:00 2001 > From: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> > Date: Thu, 8 Jun 2023 02:32:02 +0200 > Subject: [PATCH] netfilter: nf_tables: incorrect error path handling with > NFT_MSG_NEWRULE > > In case of error when adding a new rule that refers to an anonymous set, > deactivate expressions via NFT_TRANS_PREPARE state, not NFT_TRANS_RELEASE. > Thus, the lookup expression marks anonymous sets as inactive in the next > generation to ensure it is not reachable in this transaction anymore and > decrement the set refcount as introduced by c1592a89942e ("netfilter: > nf_tables: deactivate anonymous set from preparation phase"). The abort > step takes care of undoing the anonymous set. > > This is also consistent with rule deletion, where NFT_TRANS_PREPARE is > used. Note that this error path is exercised in the preparation step of > the commit protocol. This patch replaces nf_tables_rule_release() by the > deactivate and destroy calls, this time with NFT_TRANS_PREPARE. > > Due to this incorrect error handling, it is possible to access a > dangling pointer to the anonymous set that remains in the transaction > list. > > [1009.379054] BUG: KASAN: use-after-free in nft_set_lookup_global+0x147/0x1a0 [nf_tables] > [1009.379106] Read of size 8 at addr ffff88816c4c8020 by task nft-rule-add/137110 > [1009.379116] CPU: 7 PID: 137110 Comm: nft-rule-add Not tainted 6.4.0-rc4+ #256 > [1009.379128] Call Trace: > [1009.379132] <TASK> > [1009.379135] dump_stack_lvl+0x33/0x50 > [1009.379146] ? nft_set_lookup_global+0x147/0x1a0 [nf_tables] > [1009.379191] print_address_description.constprop.0+0x27/0x300 > [1009.379201] kasan_report+0x107/0x120 > [1009.379210] ? nft_set_lookup_global+0x147/0x1a0 [nf_tables] > [1009.379255] nft_set_lookup_global+0x147/0x1a0 [nf_tables] > [1009.379302] nft_lookup_init+0xa5/0x270 [nf_tables] > [1009.379350] nf_tables_newrule+0x698/0xe50 [nf_tables] > [1009.379397] ? nf_tables_rule_release+0xe0/0xe0 [nf_tables] > [1009.379441] ? kasan_unpoison+0x23/0x50 > [1009.379450] nfnetlink_rcv_batch+0x97c/0xd90 [nfnetlink] > [1009.379470] ? nfnetlink_rcv_msg+0x480/0x480 [nfnetlink] > [1009.379485] ? __alloc_skb+0xb8/0x1e0 > [1009.379493] ? __alloc_skb+0xb8/0x1e0 > [1009.379502] ? entry_SYSCALL_64_after_hwframe+0x46/0xb0 > [1009.379509] ? unwind_get_return_address+0x2a/0x40 > [1009.379517] ? write_profile+0xc0/0xc0 > [1009.379524] ? avc_lookup+0x8f/0xc0 > [1009.379532] ? __rcu_read_unlock+0x43/0x60 > > Fixes: 958bee14d071 ("netfilter: nf_tables: use new transaction infrastructure to handle sets") > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index 3bb0800b3849..69bceefaa5c8 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -3844,7 +3844,8 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info, > if (flow) > nft_flow_rule_destroy(flow); > err_release_rule: > - nf_tables_rule_release(&ctx, rule); > + nft_rule_expr_deactivate(&ctx, rule, NFT_TRANS_PREPARE); > + nf_tables_rule_destroy(&ctx, rule); > err_release_expr: > for (i = 0; i < n; i++) { > if (expr_info[i].ops) { >
