Re: [PATCH nf] netfilter: nf_tables: mark set as dead when deactivating anonymous set

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> While the rhashtable set gc runs asynchronously, a race allows it to
> collect elements from an anonymous set while it is being released from
> the abort path. This also seems possible from the rule error path.
> 
> Mingi Cho originally reported this issue in a different path in 6.1.x
> with a pipapo set with low timeouts which is not possible upstream since
> 7395dfacfff6 ("netfilter: nf_tables: use timestamp to check for set
> element timeout").
> 
> Fix this by setting on the dead flag to signal set gc to skip anonymous
> sets from prepare_error, abort and commit paths.

This seems to contradict what patch is doing, the flag gets toggled for
all set types.

> Cc: stable@vger.kernel.org
> Fixes: 5f68718b34a5 ("netfilter: nf_tables: GC transaction API to avoid race with control plane")
> Reported-by: Mingi Cho <mgcho.minic@gmail.com>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  net/netfilter/nf_tables_api.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index ca54d4c23123..26d33ce3b682 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -5513,6 +5513,7 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
>  			list_del_rcu(&binding->list);
>  
>  		nft_use_dec(&set->use);
> +		set->dead = 1;
>  		break;
>  	case NFT_TRANS_PREPARE:
>  		if (nft_set_is_anonymous(set)) {
> @@ -5534,6 +5535,7 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
>  	default:
>  		nf_tables_unbind_set(ctx, set, binding,
>  				     phase == NFT_TRANS_COMMIT);
> +		set->dead = 1;

Shouldn't that be restricted to nf_tables_unbind_set() anonymous-set
branch?