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

On Sat, Jul 25, 2026 at 4:14=E2=80=AFPM Chuck Lever <[email protected]> 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-achillesgaikwa=
[email protected]/
>
> 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.

--=20
paul-moore.com