Re: [PATCH v3] lib/rhashtable: clear stale iter->p on table restart

[NeilBrown]

On Sun, 12 Jul 2026, Herbert Xu wrote:
> On Tue, Jul 07, 2026 at 12:41:15PM -0400, Cen Zhang (Microsoft) wrote:
> > rhashtable_walk_start_check() has two restart paths when resuming a walk.
> > When iter->walker.tbl is valid, it re-validates iter->p against the table
> > and sets iter->p = NULL if the object is gone.  When iter->walker.tbl is
> > NULL (table was freed during resize), it resets slot and skip but forgets
> > to clear iter->p.
> > 
> > rhashtable_walk_next() then dereferences the stale iter->p, reading
> > freed memory.  This is a use-after-free.
> 
> Maybe I'm misreading the original patch (in the Fixes header).  But
> it seems the whole point of having it is to look for iter->p in the
> new table.  Even if the hash table remains the same iter->p could have
> been freed since we hold no reference to that object.

Certainly it could have been freed or removed etc, which is why we look
to see if it is still there.
It could even have been removed, freed, re-allocated, and reinserted in
the same chain.  In that case the behaviour is no worse than before the
patch.

Before the patch, "skip" was always used to find were we were up to.  If
something had been removed, skip would be too big and we could miss
something.  If something had been added, skip would be too small and we
could see some things twice.

After the patch we can fall-back to using skip, and if the pathological
remove/insert in same chain happens, we could see some elements twice,
which was already the case when something is added.

The key win from the patch, as described in the commit message, is that
the caller can provide a guarantee.  If it does something to ensure that
the last returned entry is *not* removed, then it can be certain that
walking is reliable with no dups or skips.  If does not make that
effort, then it gets the same guarantees as before - concurrent
add/remove and disturb the walk in unpredictable ways.

I think that patch is good and fixes a problem that needs fixing.

Reviewed-by: NeilBrown <neil@xxxxxxxxxx>

Thanks,
NeilBrown


> 
> If that is the case, then resetting iter->p on a resize doesn't
> fix this at all since the root cause is that iter->p is being
> held with no reference.
> 
> I think we should just revert the original patch since the whole
> concept doesn't seem to work (although it's salvageable for the
> non-rhlist case).
> 
> Thanks,
> -- 
> Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> 
>