[PATCH AUTOSEL 6.11 048/244] netfilter: nf_tables: do not remove elements if set backend implements .abort

[PATCH AUTOSEL 6.11 048/244] netfilter: nf_tables: do not remove elements if set backend implements .abort

[Sasha Levin]

From: Pablo Neira Ayuso <pablo@netfilter.org>

[ Upstream commit c9526aeb4998393171d85225ff540e28c7d4ab86 ]

pipapo set backend maintains two copies of the datastructure, removing
the elements from the copy that is going to be discarded slows down
the abort path significantly, from several minutes to few seconds after
this patch.

This patch was previously reverted by

  f86fb94011ae ("netfilter: nf_tables: revert do not remove elements if set backend implements .abort")

but it is now possible since recent work by Florian Westphal to perform
on-demand clone from insert/remove path:

  532aec7e878b ("netfilter: nft_set_pipapo: remove dirty flag")
  3f1d886cc7c3 ("netfilter: nft_set_pipapo: move cloning of match info to insert/removal path")
  a238106703ab ("netfilter: nft_set_pipapo: prepare pipapo_get helper for on-demand clone")
  c5444786d0ea ("netfilter: nft_set_pipapo: merge deactivate helper into caller")
  6c108d9bee44 ("netfilter: nft_set_pipapo: prepare walk function for on-demand clone")
  8b8a2417558c ("netfilter: nft_set_pipapo: prepare destroy function for on-demand clone")
  80efd2997fb9 ("netfilter: nft_set_pipapo: make pipapo_clone helper return NULL")
  a590f4760922 ("netfilter: nft_set_pipapo: move prove_locking helper around")

after this series, the clone is fully released once aborted, no need to
take it back to previous state. Thus, no stale reference to elements can
occur.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/netfilter/nf_tables_api.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0a2f793469589..ee428997a0731 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -10782,7 +10782,10 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 				break;
 			}
 			te = nft_trans_container_elem(trans);
-			nft_setelem_remove(net, te->set, te->elem_priv);
+			if (!te->set->ops->abort ||
+			    nft_setelem_is_catchall(te->set, te->elem_priv))
+				nft_setelem_remove(net, te->set, te->elem_priv);
+
 			if (!nft_setelem_is_catchall(te->set, te->elem_priv))
 				atomic_dec(&te->set->nelems);
 
-- 
2.43.0

[PATCH AUTOSEL 6.11 049/244] netfilter: nf_tables: don't initialize registers in nft_do_chain()

[Sasha Levin]

From: Florian Westphal <fw@strlen.de>

[ Upstream commit c88baabf16d1ef74ab8832de9761226406af5507 ]

revert commit 4c905f6740a3 ("netfilter: nf_tables: initialize registers in
nft_do_chain()").

Previous patch makes sure that loads from uninitialized registers are
detected from the control plane. in this case rule blob auto-zeroes
registers.  Thus the explicit zeroing is not needed anymore.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 net/netfilter/nf_tables_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index a48d5f0e2f3e1..75598520b0fa0 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -256,7 +256,7 @@ nft_do_chain(struct nft_pktinfo *pkt, void *priv)
 	const struct net *net = nft_net(pkt);
 	const struct nft_expr *expr, *last;
 	const struct nft_rule_dp *rule;
-	struct nft_regs regs = {};
+	struct nft_regs regs;
 	unsigned int stackptr = 0;
 	struct nft_jumpstack jumpstack[NFT_JUMP_STACK_SIZE];
 	bool genbit = READ_ONCE(net->nft.gencursor);
-- 
2.43.0

Re: [PATCH AUTOSEL 6.11 049/244] netfilter: nf_tables: don't initialize registers in nft_do_chain()

[Pablo Neira Ayuso]

Hi Sasha,

This commit requires:

commit 14fb07130c7ddd257e30079b87499b3f89097b09
Author: Florian Westphal <fw@strlen.de>
Date:   Tue Aug 20 11:56:13 2024 +0200

    netfilter: nf_tables: allow loads only when register is initialized

so either drop it or pull-in this dependency for 6.11

Thanks.

On Wed, Sep 25, 2024 at 07:24:30AM -0400, Sasha Levin wrote:
> From: Florian Westphal <fw@strlen.de>
> 
> [ Upstream commit c88baabf16d1ef74ab8832de9761226406af5507 ]
> 
> revert commit 4c905f6740a3 ("netfilter: nf_tables: initialize registers in
> nft_do_chain()").
> 
> Previous patch makes sure that loads from uninitialized registers are
> detected from the control plane. in this case rule blob auto-zeroes
> registers.  Thus the explicit zeroing is not needed anymore.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  net/netfilter/nf_tables_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
> index a48d5f0e2f3e1..75598520b0fa0 100644
> --- a/net/netfilter/nf_tables_core.c
> +++ b/net/netfilter/nf_tables_core.c
> @@ -256,7 +256,7 @@ nft_do_chain(struct nft_pktinfo *pkt, void *priv)
>  	const struct net *net = nft_net(pkt);
>  	const struct nft_expr *expr, *last;
>  	const struct nft_rule_dp *rule;
> -	struct nft_regs regs = {};
> +	struct nft_regs regs;
>  	unsigned int stackptr = 0;
>  	struct nft_jumpstack jumpstack[NFT_JUMP_STACK_SIZE];
>  	bool genbit = READ_ONCE(net->nft.gencursor);
> -- 
> 2.43.0
> 

Re: [PATCH AUTOSEL 6.11 049/244] netfilter: nf_tables: don't initialize registers in nft_do_chain()

[Pablo Neira Ayuso]

On Wed, Sep 25, 2024 at 01:58:54PM +0200, Pablo Neira Ayuso wrote:
> Hi Sasha,
> 
> This commit requires:
> 
> commit 14fb07130c7ddd257e30079b87499b3f89097b09
> Author: Florian Westphal <fw@strlen.de>
> Date:   Tue Aug 20 11:56:13 2024 +0200
> 
>     netfilter: nf_tables: allow loads only when register is initialized
> 
> so either drop it or pull-in this dependency for 6.11

same applies to all kernels below this 6.11

Re: [PATCH AUTOSEL 6.11 049/244] netfilter: nf_tables: don't initialize registers in nft_do_chain()

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Sasha,
> 
> This commit requires:
> 
> commit 14fb07130c7ddd257e30079b87499b3f89097b09
> Author: Florian Westphal <fw@strlen.de>
> Date:   Tue Aug 20 11:56:13 2024 +0200
> 
>     netfilter: nf_tables: allow loads only when register is initialized
> 
> so either drop it or pull-in this dependency for 6.11

It should be dropped, its crazy to pull the dependency into
stable.

Is there a way to indicate 'stable: never' in changelogs?

Re: [PATCH AUTOSEL 6.11 049/244] netfilter: nf_tables: don't initialize registers in nft_do_chain()

[Sasha Levin]

On Wed, Sep 25, 2024 at 02:20:41PM +0200, Florian Westphal wrote:
>Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> Hi Sasha,
>>
>> This commit requires:
>>
>> commit 14fb07130c7ddd257e30079b87499b3f89097b09
>> Author: Florian Westphal <fw@strlen.de>
>> Date:   Tue Aug 20 11:56:13 2024 +0200
>>
>>     netfilter: nf_tables: allow loads only when register is initialized
>>
>> so either drop it or pull-in this dependency for 6.11
>
>It should be dropped, its crazy to pull the dependency into
>stable.
>
>Is there a way to indicate 'stable: never' in changelogs?

There is a way to indicate we shouldn't pick something up with our
regular flows. See
https://docs.kernel.org/process/stable-kernel-rules.html#option-1.

-- 
Thanks,
Sasha