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

On Tue, Jul 7, 2026, at 2:48 PM, Paul Moore wrote:
> On Tue, Jul 7, 2026 at 11:24 AM Achilles Gaikwad
> <[email protected]> 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 <[email protected]>
>> Signed-off-by: Achilles Gaikwad <[email protected]>
>> ---
>> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org/
>>  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. I'll be curious to hear
what problem you're hitting, and what patch is the culprit once you
do that bisect!

Anna

>
> Reviewed-by: Paul Moore <[email protected]>
>
>> 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