Re: [PATCH] NFSv4.2: fix nfs4_listxattr NULL pointer dereference

On Fri, Jul 3, 2026 at 6:28 AM Achilles Gaikwad
<[email protected]> wrote:
>
> A call to listxattr() with a buffer size = 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() now decrements the remaining size
> even when the buffer is NULL, so in the size-query case 'left'
> underflows to a huge size_t value and nfs4_listxattr_nfs4_user()
> treats the NULL buffer as real, ending in a NULL dereference in
> _copy_from_pages().
>
> Declare 'left' as ssize_t and pass a zero length to
> nfs4_listxattr_nfs4_user() when the buffer is NULL.
>
> Fixes: f71ece9712b7 ("security,fs,nfs,net: update security_inode_listsecurity() interface")
> Signed-off-by: Achilles Gaikwad <[email protected]>
> ---
>  fs/nfs/nfs4proc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Thanks for reporting this, one thought below ...

> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 1360409d8de9..4859c2c96c78 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -10585,7 +10585,7 @@ 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;
>
>         error = generic_listxattr(dentry, list, left);
>         if (error < 0)
> @@ -10600,7 +10600,8 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
>                 return error2;
>         error2 = size - error - left;

I wonder if it would be better associate the handling code with the
security_inode_listsecurity() call a bit more closely?  Thinking about
it quickly, would something like what's below work?

  left2 = left;
  error2 = security_inode_listsecurity(..., &list, &left2);
  if (error2 < 0)
    return error2;
  error2 = left - left2;
  if (list)
    left -= error2;

> -       error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, left);
> +       error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list,
> +                                         list ? left : 0);
>         if (error3 < 0)
>                 return error3;
>
>
> base-commit: 6eb8711ece2ce27e52e327a5b7a628ed39b97f45
> --
> 2.55.0

-- 
paul-moore.com