CVE-2024-26629
Publication date:
13/03/2024
In the Linux kernel, the following vulnerability has been resolved:<br />
<br />
nfsd: fix RELEASE_LOCKOWNER<br />
<br />
The test on so_count in nfsd4_release_lockowner() is nonsense and<br />
harmful. Revert to using check_for_locks(), changing that to not sleep.<br />
<br />
First: harmful.<br />
As is documented in the kdoc comment for nfsd4_release_lockowner(), the<br />
test on so_count can transiently return a false positive resulting in a<br />
return of NFS4ERR_LOCKS_HELD when in fact no locks are held. This is<br />
clearly a protocol violation and with the Linux NFS client it can cause<br />
incorrect behaviour.<br />
<br />
If RELEASE_LOCKOWNER is sent while some other thread is still<br />
processing a LOCK request which failed because, at the time that request<br />
was received, the given owner held a conflicting lock, then the nfsd<br />
thread processing that LOCK request can hold a reference (conflock) to<br />
the lock owner that causes nfsd4_release_lockowner() to return an<br />
incorrect error.<br />
<br />
The Linux NFS client ignores that NFS4ERR_LOCKS_HELD error because it<br />
never sends NFS4_RELEASE_LOCKOWNER without first releasing any locks, so<br />
it knows that the error is impossible. It assumes the lock owner was in<br />
fact released so it feels free to use the same lock owner identifier in<br />
some later locking request.<br />
<br />
When it does reuse a lock owner identifier for which a previous RELEASE<br />
failed, it will naturally use a lock_seqid of zero. However the server,<br />
which didn&#39;t release the lock owner, will expect a larger lock_seqid and<br />
so will respond with NFS4ERR_BAD_SEQID.<br />
<br />
So clearly it is harmful to allow a false positive, which testing<br />
so_count allows.<br />
<br />
The test is nonsense because ... well... it doesn&#39;t mean anything.<br />
<br />
so_count is the sum of three different counts.<br />
1/ the set of states listed on so_stateids<br />
2/ the set of active vfs locks owned by any of those states<br />
3/ various transient counts such as for conflicting locks.<br />
<br />
When it is tested against &#39;2&#39; it is clear that one of these is the<br />
transient reference obtained by find_lockowner_str_locked(). It is not<br />
clear what the other one is expected to be.<br />
<br />
In practice, the count is often 2 because there is precisely one state<br />
on so_stateids. If there were more, this would fail.<br />
<br />
In my testing I see two circumstances when RELEASE_LOCKOWNER is called.<br />
In one case, CLOSE is called before RELEASE_LOCKOWNER. That results in<br />
all the lock states being removed, and so the lockowner being discarded<br />
(it is removed when there are no more references which usually happens<br />
when the lock state is discarded). When nfsd4_release_lockowner() finds<br />
that the lock owner doesn&#39;t exist, it returns success.<br />
<br />
The other case shows an so_count of &#39;2&#39; and precisely one state listed<br />
in so_stateid. It appears that the Linux client uses a separate lock<br />
owner for each file resulting in one lock state per lock owner, so this<br />
test on &#39;2&#39; is safe. For another client it might not be safe.<br />
<br />
So this patch changes check_for_locks() to use the (newish)<br />
find_any_file_locked() so that it doesn&#39;t take a reference on the<br />
nfs4_file and so never calls nfsd_file_put(), and so never sleeps. With<br />
this check is it safe to restore the use of check_for_locks() rather<br />
than testing so_count against the mysterious &#39;2&#39;.
Severity CVSS v4.0: Pending analysis
Last modification:
27/02/2025
