Re: [PATCH v2] NFSv4.2: fix nfs4_listxattr size accounting
Paul Moore <[email protected]>
| Newsgroups | gmane.linux.kernel.lsm,gmane.linux.nfs |
|---|---|
| Message-ID | <CAHC9VhQYjj3--K6KkDJBf6LfXqtj4TPh5LsMBpPYc0-Zz6wTMA@mail.gmail.com> |
On Tue, Jul 7, 2026 at 3:12 PM Anna Schumaker <[email protected]> wrote: > 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/[email protected]/ > >> 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