Linux NFS development
 help / color / mirror / Atom feed
* [PATCH] sunrpc: prevent out-of-bounds read in __cache_seq_start()
@ 2026-04-21 16:11 Chuck Lever
  2026-04-21 20:33 ` Benjamin Coddington
  2026-04-22  2:46 ` NeilBrown
  0 siblings, 2 replies; 3+ messages in thread
From: Chuck Lever @ 2026-04-21 16:11 UTC (permalink / raw)
  To: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, Chuck Lever, syzbot+60cfa08822470bbebe44

From: Chuck Lever <chuck.lever@oracle.com>

Commit 7b546bd89975 ("sunrpc/cache: improve RCU safety in
cache_list walking.") changed the tail of __cache_seq_start()
to unconditionally store

	*pos = ((long long)hash << 32) + 1

before returning, dropping a prior "hash >= cd->hash_size"
guard. When the while loop exits because every remaining
bucket was empty, hash equals cd->hash_size, so the stored
*pos is one position past the table's last valid bucket.
seq_read_iter() caches that index in m->index. A subsequent
pread(2) at the same file offset skips traverse() and hands
the stored index back to __cache_seq_start(), which decodes
hash = cd->hash_size and dereferences
cd->hash_table[cd->hash_size] -- one hlist_head past the end
of the kzalloc'd table.

KASAN reports an eight-byte slab-out-of-bounds read at the
tail of the 2048-byte hash_table allocation for the NFSD
export cache (EXPORT_HASHMAX * sizeof(struct hlist_head) ==
256 * 8).

Reject an input hash that is out of range before touching the
hash table. cache_seq_next() already bounds-checks its own
loop; the start routine needs to be symmetric.

Reported-by: syzbot+60cfa08822470bbebe44@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=60cfa08822470bbebe44
Fixes: 7b546bd89975 ("sunrpc/cache: improve RCU safety in cache_list walking.")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/cache.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 305c6e67f052..391037f15292 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1352,6 +1352,9 @@ static void *__cache_seq_start(struct seq_file *m, loff_t *pos)
 	hash = n >> 32;
 	entry = n & ((1LL<<32) - 1);
 
+	if (hash >= cd->hash_size)
+		return NULL;
+
 	hlist_for_each_entry_rcu(ch, &cd->hash_table[hash], cache_list)
 		if (!entry--)
 			return ch;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] sunrpc: prevent out-of-bounds read in __cache_seq_start()
  2026-04-21 16:11 [PATCH] sunrpc: prevent out-of-bounds read in __cache_seq_start() Chuck Lever
@ 2026-04-21 20:33 ` Benjamin Coddington
  2026-04-22  2:46 ` NeilBrown
  1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Coddington @ 2026-04-21 20:33 UTC (permalink / raw)
  To: Chuck Lever
  Cc: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
	linux-nfs, Chuck Lever, syzbot+60cfa08822470bbebe44

On 21 Apr 2026, at 12:11, Chuck Lever wrote:

> From: Chuck Lever <chuck.lever@oracle.com>
>
> Commit 7b546bd89975 ("sunrpc/cache: improve RCU safety in
> cache_list walking.") changed the tail of __cache_seq_start()
> to unconditionally store
>
> 	*pos = ((long long)hash << 32) + 1
>
> before returning, dropping a prior "hash >= cd->hash_size"
> guard. When the while loop exits because every remaining
> bucket was empty, hash equals cd->hash_size, so the stored
> *pos is one position past the table's last valid bucket.
> seq_read_iter() caches that index in m->index. A subsequent
> pread(2) at the same file offset skips traverse() and hands
> the stored index back to __cache_seq_start(), which decodes
> hash = cd->hash_size and dereferences
> cd->hash_table[cd->hash_size] -- one hlist_head past the end
> of the kzalloc'd table.
>
> KASAN reports an eight-byte slab-out-of-bounds read at the
> tail of the 2048-byte hash_table allocation for the NFSD
> export cache (EXPORT_HASHMAX * sizeof(struct hlist_head) ==
> 256 * 8).
>
> Reject an input hash that is out of range before touching the
> hash table. cache_seq_next() already bounds-checks its own
> loop; the start routine needs to be symmetric.
>
> Reported-by: syzbot+60cfa08822470bbebe44@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=60cfa08822470bbebe44
> Fixes: 7b546bd89975 ("sunrpc/cache: improve RCU safety in cache_list walking.")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

Thanks!!
We were hunting for this one.  You'll probably get a Tested-by shortly.

Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com>

Ben

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] sunrpc: prevent out-of-bounds read in __cache_seq_start()
  2026-04-21 16:11 [PATCH] sunrpc: prevent out-of-bounds read in __cache_seq_start() Chuck Lever
  2026-04-21 20:33 ` Benjamin Coddington
@ 2026-04-22  2:46 ` NeilBrown
  1 sibling, 0 replies; 3+ messages in thread
From: NeilBrown @ 2026-04-22  2:46 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs,
	Chuck Lever, syzbot+60cfa08822470bbebe44

On Wed, 22 Apr 2026, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Commit 7b546bd89975 ("sunrpc/cache: improve RCU safety in
> cache_list walking.") changed the tail of __cache_seq_start()
> to unconditionally store
> 
> 	*pos = ((long long)hash << 32) + 1
> 
> before returning, dropping a prior "hash >= cd->hash_size"
> guard. When the while loop exits because every remaining
> bucket was empty, hash equals cd->hash_size, so the stored
> *pos is one position past the table's last valid bucket.
> seq_read_iter() caches that index in m->index. A subsequent
> pread(2) at the same file offset skips traverse() and hands
> the stored index back to __cache_seq_start(), which decodes
> hash = cd->hash_size and dereferences
> cd->hash_table[cd->hash_size] -- one hlist_head past the end
> of the kzalloc'd table.
> 
> KASAN reports an eight-byte slab-out-of-bounds read at the
> tail of the 2048-byte hash_table allocation for the NFSD
> export cache (EXPORT_HASHMAX * sizeof(struct hlist_head) ==
> 256 * 8).
> 
> Reject an input hash that is out of range before touching the
> hash table. cache_seq_next() already bounds-checks its own
> loop; the start routine needs to be symmetric.
> 
> Reported-by: syzbot+60cfa08822470bbebe44@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=60cfa08822470bbebe44
> Fixes: 7b546bd89975 ("sunrpc/cache: improve RCU safety in cache_list walking.")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

Reviewed-by: NeilBrown <neil@brown.name>

Thanks for finding and fixing this.
We could of course avoid ever storing a too-large pos but adding
back a test for hash at the end of __cache_seq_start() but I prefer
the approach you took as it is more robust.

Thanks,
NeilBrown


> ---
>  net/sunrpc/cache.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 305c6e67f052..391037f15292 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -1352,6 +1352,9 @@ static void *__cache_seq_start(struct seq_file *m, loff_t *pos)
>  	hash = n >> 32;
>  	entry = n & ((1LL<<32) - 1);
>  
> +	if (hash >= cd->hash_size)
> +		return NULL;
> +
>  	hlist_for_each_entry_rcu(ch, &cd->hash_table[hash], cache_list)
>  		if (!entry--)
>  			return ch;
> -- 
> 2.53.0
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-22  2:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 16:11 [PATCH] sunrpc: prevent out-of-bounds read in __cache_seq_start() Chuck Lever
2026-04-21 20:33 ` Benjamin Coddington
2026-04-22  2:46 ` NeilBrown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox