Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> Fixes: 5f68718b34a5 ("netfilter: nf_tables: GC transaction API to avoid race with control plane")
> Reported-by: Mingi Cho <mgcho.minic@xxxxxxxxx>
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> 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?
