Message ID | 20250512030252.3329782-1-jianqi.ren.cn@windriver.com |
---|---|
State | Handled Elsewhere, archived |
Headers | show |
Series | [6.1.y] netfilter: nf_tables: fix memleak in map from abort path | expand |
Hi, NACK. This patch requires: e79b47a8615d ("netfilter: nf_tables: restore set elements when delete set fails") which you have skipped for some reason. On Mon, May 12, 2025 at 11:02:52AM +0800, jianqi.ren.cn@windriver.com wrote: > From: Pablo Neira Ayuso <pablo@netfilter.org> > > [ Upstream commit 86a1471d7cde792941109b93b558b5dc078b9ee9 ] > > The delete set command does not rely on the transaction object for > element removal, therefore, a combination of delete element + delete set > from the abort path could result in restoring twice the refcount of the > mapping. > > Check for inactive element in the next generation for the delete element > command in the abort path, skip restoring state if next generation bit > has been already cleared. This is similar to the activate logic using > the set walk iterator. > > [ 6170.286929] ------------[ cut here ]------------ > [ 6170.286939] WARNING: CPU: 6 PID: 790302 at net/netfilter/nf_tables_api.c:2086 nf_tables_chain_destroy+0x1f7/0x220 [nf_tables] > [ 6170.287071] Modules linked in: [...] > [ 6170.287633] CPU: 6 PID: 790302 Comm: kworker/6:2 Not tainted 6.9.0-rc3+ #365 > [ 6170.287768] RIP: 0010:nf_tables_chain_destroy+0x1f7/0x220 [nf_tables] > [ 6170.287886] Code: df 48 8d 7d 58 e8 69 2e 3b df 48 8b 7d 58 e8 80 1b 37 df 48 8d 7d 68 e8 57 2e 3b df 48 8b 7d 68 e8 6e 1b 37 df 48 89 ef eb c4 <0f> 0b 48 83 c4 08 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 0f > [ 6170.287895] RSP: 0018:ffff888134b8fd08 EFLAGS: 00010202 > [ 6170.287904] RAX: 0000000000000001 RBX: ffff888125bffb28 RCX: dffffc0000000000 > [ 6170.287912] RDX: 0000000000000003 RSI: ffffffffa20298ab RDI: ffff88811ebe4750 > [ 6170.287919] RBP: ffff88811ebe4700 R08: ffff88838e812650 R09: fffffbfff0623a55 > [ 6170.287926] R10: ffffffff8311d2af R11: 0000000000000001 R12: ffff888125bffb10 > [ 6170.287933] R13: ffff888125bffb10 R14: dead000000000122 R15: dead000000000100 > [ 6170.287940] FS: 0000000000000000(0000) GS:ffff888390b00000(0000) knlGS:0000000000000000 > [ 6170.287948] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 6170.287955] CR2: 00007fd31fc00710 CR3: 0000000133f60004 CR4: 00000000001706f0 > [ 6170.287962] Call Trace: > [ 6170.287967] <TASK> > [ 6170.287973] ? __warn+0x9f/0x1a0 > [ 6170.287986] ? nf_tables_chain_destroy+0x1f7/0x220 [nf_tables] > [ 6170.288092] ? report_bug+0x1b1/0x1e0 > [ 6170.287986] ? nf_tables_chain_destroy+0x1f7/0x220 [nf_tables] > [ 6170.288092] ? report_bug+0x1b1/0x1e0 > [ 6170.288104] ? handle_bug+0x3c/0x70 > [ 6170.288112] ? exc_invalid_op+0x17/0x40 > [ 6170.288120] ? asm_exc_invalid_op+0x1a/0x20 > [ 6170.288132] ? nf_tables_chain_destroy+0x2b/0x220 [nf_tables] > [ 6170.288243] ? nf_tables_chain_destroy+0x1f7/0x220 [nf_tables] > [ 6170.288366] ? nf_tables_chain_destroy+0x2b/0x220 [nf_tables] > [ 6170.288483] nf_tables_trans_destroy_work+0x588/0x590 [nf_tables] > > Fixes: 591054469b3e ("netfilter: nf_tables: revisit chain/object refcounting from elements") > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > [fixed conflicts due to missing commits > 0e1ea651c9717ddcd8e0648d8468477a31867b0a ("netfilter: nf_tables: shrink > memory consumption of set elements") and > 9dad402b89e81a0516bad5e0ac009b7a0a80898f ("netfilter: nf_tables: expose > opaque set element as struct nft_elem_priv") so we pass the correct types > and values to nft_setelem_active_next() + nft_set_elem_ext()] > Signed-off-by: Jianqi Ren <jianqi.ren.cn@windriver.com> > Signed-off-by: He Zhe <zhe.he@windriver.com> > --- > Verified the build test > --- > net/netfilter/nf_tables_api.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > index 656c4fb76773..1d4d77d21d61 100644 > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -6772,6 +6772,16 @@ void nft_data_hold(const struct nft_data *data, enum nft_data_types type) > } > } > > +static int nft_setelem_active_next(const struct net *net, > + const struct nft_set *set, > + struct nft_set_elem *elem) > +{ > + const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); > + u8 genmask = nft_genmask_next(net); > + > + return nft_set_elem_active(ext, genmask); > +} > + > static void nft_setelem_data_activate(const struct net *net, > const struct nft_set *set, > struct nft_set_elem *elem) > @@ -10115,8 +10125,10 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action) > case NFT_MSG_DELSETELEM: > te = (struct nft_trans_elem *)trans->data; > > - nft_setelem_data_activate(net, te->set, &te->elem); > - nft_setelem_activate(net, te->set, &te->elem); > + if (!nft_setelem_active_next(net, te->set, &te->elem)) { > + nft_setelem_data_activate(net, te->set, &te->elem); > + nft_setelem_activate(net, te->set, &te->elem); > + } > if (!nft_setelem_is_catchall(te->set, &te->elem)) > te->set->ndeact--; > > -- > 2.34.1 >
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 656c4fb76773..1d4d77d21d61 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -6772,6 +6772,16 @@ void nft_data_hold(const struct nft_data *data, enum nft_data_types type) } } +static int nft_setelem_active_next(const struct net *net, + const struct nft_set *set, + struct nft_set_elem *elem) +{ + const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); + u8 genmask = nft_genmask_next(net); + + return nft_set_elem_active(ext, genmask); +} + static void nft_setelem_data_activate(const struct net *net, const struct nft_set *set, struct nft_set_elem *elem) @@ -10115,8 +10125,10 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action) case NFT_MSG_DELSETELEM: te = (struct nft_trans_elem *)trans->data; - nft_setelem_data_activate(net, te->set, &te->elem); - nft_setelem_activate(net, te->set, &te->elem); + if (!nft_setelem_active_next(net, te->set, &te->elem)) { + nft_setelem_data_activate(net, te->set, &te->elem); + nft_setelem_activate(net, te->set, &te->elem); + } if (!nft_setelem_is_catchall(te->set, &te->elem)) te->set->ndeact--;