CVE-2024-26629

In the Linux kernel, the following vulnerability has been resolved: nfsd:
fix RELEASE_LOCKOWNER The test on so_count in nfsd4_release_lockowner() is
nonsense and harmful. Revert to using check_for_locks(), changing that to
not sleep. First: harmful. As is documented in the kdoc comment for
nfsd4_release_lockowner(), the test on so_count can transiently return a
false positive resulting in a return of NFS4ERR_LOCKS_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 RELEASE_LOCKOWNER 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
nfsd4_release_lockowner() to return an incorrect error. The Linux NFS
client ignores that NFS4ERR_LOCKS_HELD error because it never sends
NFS4_RELEASE_LOCKOWNER 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 lock_seqid of zero. However the
server, which didn’t release the lock owner, will expect a larger
lock_seqid and so will respond with NFS4ERR_BAD_SEQID. 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. so_count is the
sum of three different counts. 1/ the set of states listed on so_stateids
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
find_lockowner_str_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 RELEASE_LOCKOWNER is called. In one case, CLOSE
is called before RELEASE_LOCKOWNER. 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 nfsd4_release_lockowner() finds that the lock owner
doesn’t exist, it returns success. The other case shows an so_count of ‘2’
and precisely one state listed in so_stateid. 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 check_for_locks() to use the (newish)
find_any_file_locked() so that it doesn’t take a reference on the nfs4_file
and so never calls nfsd_file_put(), and so never sleeps. With this check is
it safe to restore the use of check_for_locks() rather than testing
so_count against the mysterious ‘2’.

Notes