CVE-2024-26629

In the Linux kernel, the following vulnerability has been resolved:

nfsd: fix RELEASE_LOCKOWNER

The test on socount in nfsd4releaselockowner() is nonsense and harmful. Revert to using checkfor_locks(), changing that to not sleep.

First: harmful. As is documented in the kdoc comment for nfsd4releaselockowner(), the test on socount can transiently return a false positive resulting in a return of NFS4ERRLOCKS_HELD when in fact no locks are held. This is clearly a protocol violation and with the Linux NFS client it can cause incorrect behaviour.

If RELEASELOCKOWNER is sent while some other thread is still processing a LOCK request which failed because, at the time that request was received, the given owner held a conflicting lock, then the nfsd thread processing that LOCK request can hold a reference (conflock) to the lock owner that causes nfsd4release_lockowner() to return an incorrect error.

The Linux NFS client ignores that NFS4ERRLOCKSHELD error because it never sends NFS4RELEASELOCKOWNER without first releasing any locks, so it knows that the error is impossible. It assumes the lock owner was in fact released so it feels free to use the same lock owner identifier in some later locking request.

When it does reuse a lock owner identifier for which a previous RELEASE failed, it will naturally use a lockseqid of zero. However the server, which didn't release the lock owner, will expect a larger lockseqid and so will respond with NFS4ERRBADSEQID.

So clearly it is harmful to allow a false positive, which testing so_count allows.

The test is nonsense because ... well... it doesn't mean anything.

socount is the sum of three different counts. 1/ the set of states listed on sostateids 2/ the set of active vfs locks owned by any of those states 3/ various transient counts such as for conflicting locks.

When it is tested against '2' it is clear that one of these is the transient reference obtained by findlockownerstr_locked(). It is not clear what the other one is expected to be.

In practice, the count is often 2 because there is precisely one state on so_stateids. If there were more, this would fail.

In my testing I see two circumstances when RELEASELOCKOWNER is called. In one case, CLOSE is called before RELEASELOCKOWNER. That results in all the lock states being removed, and so the lockowner being discarded (it is removed when there are no more references which usually happens when the lock state is discarded). When nfsd4releaselockowner() finds that the lock owner doesn't exist, it returns success.

The other case shows an socount of '2' and precisely one state listed in sostateid. It appears that the Linux client uses a separate lock owner for each file resulting in one lock state per lock owner, so this test on '2' is safe. For another client it might not be safe.

So this patch changes checkforlocks() to use the (newish) findanyfilelocked() so that it doesn't take a reference on the nfs4file and so never calls nfsdfileput(), and so never sleeps. With this check is it safe to restore the use of checkforlocks() rather than testing so_count against the mysterious '2'.