Re: [PATCH v2] security,fs,nfs,net: update security_inode_listsecurity() interface

["Anna Schumaker" <anna@kernel.org>]

On Fri, Jul 31, 2026, at 1:07 PM, Chuck Lever wrote:
> On Fri, Jul 31, 2026, at 12:09 PM, Paul Moore wrote:
>> On Sun, Jul 26, 2026 at 11:03 AM Paul Moore <paul@paul-moore.com> wrote:
>>> On Sat, Jul 25, 2026 at 4:14 PM Chuck Lever <cel@kernel.org> wrote:
>>> >
>>> > On Mon, Apr 28, 2025 at 03:50:19PM -0400, Stephen Smalley wrote:
>>> > > Update the security_inode_listsecurity() interface to allow
>>> > > use of the xattr_list_one() helper and update the hook
>>> > > implementations.
>>> >
>>> > This commit wedges an NFSv4.2 client running fstests.
>>> >
>>> > A kdevops fstests run against nfsd-next hangs at generic/086.  The
>>> > client stops making progress and never recovers:
>>> >
>>> >   watchdog: BUG: soft lockup - CPU#3 stuck for 250s! [086:60396]
>>> >   Comm: 086 Tainted: G  D  L  7.2.0-rc4
>>> >     __pv_queued_spin_lock_slowpath
>>> >     _raw_spin_lock
>>> >     nfs4_xattr_set_listcache [nfsv4]
>>> >     nfs4_xattr_discard_cache [nfsv4]
>>> >     nfs4_xattr_cache_scan [nfsv4]
>>> >     do_shrink_slab
>>> >     drop_caches_sysctl_handler
>>> >
>>> > The D taint says something died earlier.  It did: by the time the
>>> > lockup fires, the client has already taken two dozen NULL pointer
>>> > dereferences, all of them in the same place:
>>> >
>>> >   BUG: kernel NULL pointer dereference, address: 0000000000000000
>>> >   RIP: _copy_from_pages+0x44/0xd0 [sunrpc]
>>> >     nfs4_xattr_cache_list [nfsv4]
>>> >     nfs4_listxattr [nfsv4]
>>> >     vfs_listxattr
>>> >     __x64_sys_listxattr
>>> >
>>> > The two stages are connected.  nfs4_listxattr() declares its
>>> > remaining-size accumulator as an unsigned size_t.  On a size query,
>>> > where listxattr(2) passes a NULL buffer and a length of zero,
>>> > xattr_list_one() decrements that accumulator for the SELinux label
>>> > without regard to the NULL buffer, so it wraps to a very large
>>> > value.  nfs4_xattr_cache_list() then sees a NULL buffer paired with
>>> > a buffer length large enough to pass its own bounds check, and
>>> > copies into it.  The fault happens while the xattr listcache
>>> > spinlock is held.  Task exit does not release spinlocks, so the
>>> > lock is orphaned, and the next shrinker pass spins on it until the
>>> > watchdog fires.
>>> >
>>> > The reproducer is a plain getfattr on a file on an NFSv4.2 mount
>>> > with SELinux enforcing, which is what fstests generic/086 arrives
>>> > at by way of a drop_caches write.  No fstests machinery is needed
>>> > to see the oops.
>>> >
>>> > An audit of the listxattr path pointed here, so I built nfsd-next
>>> > with this commit reverted and reran the full fstests nfs_v42
>>> > section on the same pair of guests.  745 tests, no soft lockup, no
>>> > oops, and generic/086 passes.  The two failures that remain,
>>> > generic/033 and generic/258, are unrelated to xattrs.
>>> >
>>> > I am not proposing the revert as the fix.  Achilles Gaikwad has
>>> > already posted a targeted patch for the accounting itself:
>>> >
>>> >   https://lore.kernel.org/linux-nfs/20260707152305.15324-1-achillesgaikwad@gmail.com/
>>> >
>>> > Whether the right answer is that patch, a change to
>>> > xattr_list_one() so it leaves the remaining size alone when the
>>> > buffer is NULL, or hardening nfs4_xattr_cache_list() against a NULL
>>> > buffer, is for the three of you to settle.  I am reporting the
>>> > severity, because the accounting bug is not confined to a short
>>> > size query: on a filesystem that caches xattrs behind a lock, it
>>> > takes the whole machine down.
>>> >
>>> > I can test whatever you settle on against the same setup.
>>>
>>> I reviewed Achilles's patch back in early July and it looked okay to
>>> me (see my tag in my email in that thread from July 7th).  I had
>>> figured that the NFS folks would have wanted to pull that via the NFS
>>> tree since it only touches NFS code.  If they would prefer I send it
>>> up to Linus via the LSM tree I can do that, but I would appreciate an
>>> ACK and Tested-by from someone on the NFS side.
>>>
>>> While I don't suspect it will be very interesting, I just started
>>> building a test kernel with Achilles's patch that I'll run through the
>>> normal selinux-testsuite automated testing.
>>
>> Chuck, Anna, Trond, Jeff - can someone ACK Achilles's patch below or
>> suggest what changes might be necessary?  There is a known regression
>> (see the existing mails) and we need to fix this before v7.2 ships.
>> I'm happy to send this up to Linus if you would prefer, but I would
>> *really* like to see an ACK from someone on the NFS side.  If I don't
>> see any comments be early next week I'll send this up to Linus via the
>> LSM tree.
>>
>> https://lore.kernel.org/linux-nfs/20260707152305.15324-1-achillesgaikwad@gmail.com/
>
> Since Achilles' patch is client-side, I shouldn't Ack it.
>
> I don't see this patch in Anna's tree either. A response from the
> NFS maintainers would be helpful.

Looks like I did have it in a branch with other commits for testing, but I
never pushed it out. I've moved it over to my public linux-next and will
pass it along to Linus in the next few days.

Anna

>
>
> -- 
> Chuck Lever