Thread (9 messages) 9 messages, 3 authors, 1d ago

Re: [PATCH v2] NFSv4.2: fix nfs4_listxattr size accounting

From: Paul Moore <paul@paul-moore.com>
Date: 2026-07-07 20:01:55
Also in: linux-nfs, selinux

On Tue, Jul 7, 2026 at 3:12 PM Anna Schumaker [off-list ref] wrote:
On Tue, Jul 7, 2026, at 2:48 PM, Paul Moore wrote:
quoted
On Tue, Jul 7, 2026 at 11:24 AM Achilles Gaikwad
[off-list ref] wrote:
quoted
A call to listxattr() with a buffer size of 0 returns the actual
size of the buffer needed for a subsequent call. On an NFSv4.2
mount this triggers the following oops:

  [  399.768687] BUG: kernel NULL pointer dereference, address: 0000000000000000
  [  399.768705] RIP: 0010:_copy_from_pages+0x44/0xe0
  [  399.768722] Call Trace:
  [  399.768723]  nfs4_xattr_alloc_entry+0x1bf/0x1e0
  [  399.768730]  nfs4_xattr_cache_set_list+0x43/0x1f0
  [  399.768731]  nfs4_listxattr+0x21f/0x250
  [  399.768733]  vfs_listxattr+0x55/0xa0
  [  399.768736]  listxattr+0x23/0x160
  [  399.768737]  path_listxattrat+0xba/0x1e0
  [  399.768739]  do_syscall_64+0xe2/0x680

security_inode_listsecurity() (via the xattr_list_one() helper) now
decrements the remaining size even when the buffer pointer is NULL, so
in the size-query case, 'left' underflows to a huge size_t value. As a
result, nfs4_listxattr_nfs4_user() treats the NULL buffer as a real one,
leading to a NULL pointer dereference in _copy_from_pages().

security_inode_listsecurity() does not return the number of bytes
it added to the list, so the code derived it as
'size - error - left'. That is also wrong in the size-query case:
the generic_listxattr() contribution is only subtracted from 'left'
when a buffer is present. Thus, the query result comes up short by
exactly that contribution (e.g., "system.nfs4_acl" on a mount with
ACL support), and a caller that allocates the returned size gets
-ERANGE on the subsequent call.

Declare 'left' as ssize_t, use a scratch copy to measure security
hook consumption, and only decrement 'left' if a buffer is present.

Fixes: f71ece9712b7 ("security,fs,nfs,net: update security_inode_listsecurity() interface")
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Achilles Gaikwad <redacted>
---
Changes in v2:
 - Use a scratch variable to track security label size directly,
   replacing the old formula that undercounted the size-query case.
 - Drop the now-unneeded NULL-buffer special case for
   nfs4_listxattr_nfs4_user().
 - Retitled from "fix nfs4_listxattr NULL pointer dereference"
   (the same accounting bug caused both the oops and the undercount).
v1: https://lore.kernel.org/linux-nfs/20260703102759.9626-1-achillesgaikwad@gmail.com/ (local)
 fs/nfs/nfs4proc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
[CC'd the LSM and SELinux lists for visibility]

Unfortunately my testing was unsuccessful due to an NFS problem that
started with the v7.2 merge window that I haven't had the time to
bisect yet.  Assuming the NFS folks are okay with this change, I
figure they will want to send it up to Linus via their tree, if not
let me know and I can send this up via the LSM tree.
Yeah, we'll send it through the NFS tree.
Thanks Anna.
I'll be curious to hear
what problem you're hitting, and what patch is the culprit once you
do that bisect!
Yes, me too :)

I'm still working through a review backlog so it might be a bit before
I have a chance, but in case anyone wants to test it out, it's easily
reproduced using the selinux-testsuite and the NFS tests:

https://github.com/SELinuxProject/selinux-testsuite#nfs

-- 
paul-moore.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help