Re: [PATCH v2] NFSv4.2: fix nfs4_listxattr size accounting
From: Paul Moore <paul@paul-moore.com>
Date: 2026-07-07 18:48:26
Also in:
linux-nfs, selinux
On Tue, Jul 7, 2026 at 11:24 AM Achilles Gaikwad [off-list ref] wrote:
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. Reviewed-by: Paul Moore <paul@paul-moore.com>
quoted hunk
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 1360409d8de9..a3415082d610 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c@@ -10585,7 +10585,8 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = { static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size) { ssize_t error, error2, error3; - size_t left = size; + ssize_t left = size; + ssize_t left2; error = generic_listxattr(dentry, list, left); if (error < 0)@@ -10595,10 +10596,13 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size) left -= error; } - error2 = security_inode_listsecurity(d_inode(dentry), &list, &left); + left2 = left; + error2 = security_inode_listsecurity(d_inode(dentry), &list, &left2); if (error2 < 0) return error2; - error2 = size - error - left; + error2 = left - left2; + if (list) + left -= error2; error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, left); if (error3 < 0)base-commit: 6eb8711ece2ce27e52e327a5b7a628ed39b97f45 -- 2.55.0
-- paul-moore.com