On Tue, 9 Apr 2024 15:04:02 +0200 Florian Westphal <fw@xxxxxxxxx> wrote: > Stefano Brivio <sbrivio@xxxxxxxxxx> wrote: > > > I do not follow. nft_pipapo_destroy() is not invoked asynchronously via > > > call_rcu, its invoked from either abort path or the gc work queue at at > > > time where there must be no references to nft_set anymore. > > > > Hmm, sorry, I was all focused on nft_set_pipapo_match_destroy() > > accessing nft_set, but that has nothing to do with > > pipapo_reclaim_match(). However: > > > > > What do we wait for, i.e., which outstanding rcu callback could > > > reference a data structure that nft_pipapo_destroy() will free? > > > > ...we still have pipapo_free_match(), called by pipapo_reclaim_match(), > > referencing the per-CPU scratch areas, and nft_pipapo_destroy() freeing > > them (using pipapo_free_match() since this patch). > > But those scratchmaps are anchored in struct nft_pipapo_match. > > So, if we have a call_rcu() for struct nft_pipapo_match $m, and then > get into nft_pipapo_destroy() where priv->match == $m or > priv->clone == $m we are already in trouble ($m is free'd twice). Ah, sure, you're right, they can't be the same instance. > If not, then I don't see why ordering would matter. > > Can you sketch a race where pipapo_reclaim_match, running from a > (severely delayed) call_rcu, will access something that has been > released already? > > I can't spot anything. Me neither. Sorry for the noise. For the series, Reviewed-by: Stefano Brivio <sbrivio@xxxxxxxxxx> -- Stefano
