Re: [PATCH v2] security,fs,nfs,net: update security_inode_listsecurity() interface
Paul Moore <[email protected]> Fri, 31 Jul 2026 12:09:36 -0400
| Newsgroups | org.kernel.vger.linux-nfs,org.kernel.vger.linux-security-module,org.kernel.vger.selinux |
|---|---|
| Message-ID | <CAHC9VhS6B1yW+8G9Biqit5CAmcg4CwCaWbq1B+Qpo2n6a=yqnw@mail.gmail.com> |
On Sun, Jul 26, 2026 at 11:03=E2=80=AFAM Paul Moore <[email protected]> w= rote: > On Sat, Jul 25, 2026 at 4:14=E2=80=AFPM Chuck Lever <[email protected]> wrot= e: > > > > 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-achillesgaik= [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. 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@gm= ail.com/ --=20 paul-moore.com