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

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

[Cen Zhang (Microsoft)]

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.

Any caller that does multi-fragment rhashtable walks across
walk_stop/walk_start boundaries is affected.  Concrete cases include
netlink_diag (__netlink_diag_dump in net/netlink/diag.c) and TIPC
(tipc_nl_sk_walk in net/tipc/socket.c).

Crash stack (netlink_diag):
  BUG: KASAN: slab-use-after-free in rhashtable_walk_next+0x365/0x3c0
  Read of size 8 at addr ffff88801a9d2438 (freed kmalloc-2k, offset 1080)
  Call Trace:
   rhashtable_walk_next+0x365/0x3c0 (lib/rhashtable.c:1016)
   __netlink_diag_dump+0x160/0x760 (net/netlink/diag.c:122)
   netlink_diag_dump+0xc2/0x240
   netlink_dump+0x5bc/0x1270
   netlink_recvmsg+0x7a3/0x980
   sock_recvmsg+0x1bc/0x200
   __sys_recvfrom+0x1d4/0x2c0

Fixes: 5d240a8936f6 ("rhashtable: improve rhashtable_walk stability when
stop/start used.")
Reported-by: AutonomousCodeSecurity@microsoft.com
Closes: https://lore.kernel.org/linux-crypto/CAB8m9Wh559e+=n8z51gB8DrbEyCc2mc0MgGjrRR6_VXBmU=2AQ@mail.gmail.com
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
v3: Solved patch format issue
v2: Fix commit subject in Fixes tag
Link: https://lore.kernel.org/linux-crypto/CAB8m9Wh559e+=n8z51gB8DrbEyCc2mc0MgGjrRR6_VXBmU=2AQ@mail.gmail.com

 lib/rhashtable.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 40cfb38ac..d459bef24 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -878,6 +878,7 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
 		iter->walker.tbl = rht_dereference_rcu(ht->tbl, ht);
 		iter->slot = 0;
 		iter->skip = 0;
+		iter->p = NULL;
 		return -EAGAIN;
 	}
 
-- 
2.53.0

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

[Herbert Xu]

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.

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@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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@brown.name>

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@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> 
> 

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

[Herbert Xu]

On Mon, Jul 13, 2026 at 02:55:31PM +1000, NeilBrown wrote:
>
> I think that patch is good and fixes a problem that needs fixing.

Oh I get it now.  It was rhashtable_walk_next that was dereferencing
iter->p, not rhashtable_walk_start_check.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Herbert Xu]

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.
> 
> Any caller that does multi-fragment rhashtable walks across
> walk_stop/walk_start boundaries is affected.  Concrete cases include
> netlink_diag (__netlink_diag_dump in net/netlink/diag.c) and TIPC
> (tipc_nl_sk_walk in net/tipc/socket.c).
> 
> Crash stack (netlink_diag):
>   BUG: KASAN: slab-use-after-free in rhashtable_walk_next+0x365/0x3c0
>   Read of size 8 at addr ffff88801a9d2438 (freed kmalloc-2k, offset 1080)
>   Call Trace:
>    rhashtable_walk_next+0x365/0x3c0 (lib/rhashtable.c:1016)
>    __netlink_diag_dump+0x160/0x760 (net/netlink/diag.c:122)
>    netlink_diag_dump+0xc2/0x240
>    netlink_dump+0x5bc/0x1270
>    netlink_recvmsg+0x7a3/0x980
>    sock_recvmsg+0x1bc/0x200
>    __sys_recvfrom+0x1d4/0x2c0
> 
> Fixes: 5d240a8936f6 ("rhashtable: improve rhashtable_walk stability when
> stop/start used.")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Closes: https://lore.kernel.org/linux-crypto/CAB8m9Wh559e+=n8z51gB8DrbEyCc2mc0MgGjrRR6_VXBmU=2AQ@mail.gmail.com
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> ---
> v3: Solved patch format issue
> v2: Fix commit subject in Fixes tag
> Link: https://lore.kernel.org/linux-crypto/CAB8m9Wh559e+=n8z51gB8DrbEyCc2mc0MgGjrRR6_VXBmU=2AQ@mail.gmail.com
> 
>  lib/rhashtable.c | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt