[PATCH v1] nfsd: Fix race between FREE_STATEID and LOCK

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Using LTP's nfslock01 test, one of our internal testers found that
the Linux client can send a LOCK and a FREE_STATEID request at the
same time. The LOCK uses the same lockowner as the stateid sent in
the FREE_STATEID request.

The outcome is:

Frame 115025 C FREE_STATEID stateid 2/A
Frame 115026 C LOCK offset 672128 len 64
Frame 115029 R FREE_STATEID NFS4_OK
Frame 115030 R LOCK stateid 3/A
Frame 115034 C WRITE stateid 0/A offset 672128 len 64
Frame 115038 R WRITE NFS4ERR_BAD_STATEID

In other words, the server returns stateid A in the LOCK reply, but
it has already released it. Subsequent uses of the stateid fail.

To address this, protect the logic in nfsd4_free_stateid with the
st_mutex. This should guarantee that only one of two outcomes
occurs: either LOCK returns a fresh valid stateid, or FREE_STATEID
returns NFS4ERR_LOCKS_HELD.

Reported-by: Alexey Kodanev <alexey.kodanev@xxxxxxxxxx>
Fix-suggested-by: Jeff Layton <jlayton@xxxxxxxxxx>
Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
---

Before I pass this along to Alexey for testing, I'd appreciate some
review of the proposed fix.


 fs/nfsd/nfs4state.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b921123..a9e0606 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4911,16 +4911,24 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		ret = nfserr_locks_held;
 		break;
 	case NFS4_LOCK_STID:
-		ret = check_stateid_generation(stateid, &s->sc_stateid, 1);
-		if (ret)
-			break;
+		spin_unlock(&cl->cl_lock);
 		stp = openlockstateid(s);
+		mutex_lock(&stp->st_mutex);
+		ret = check_stateid_generation(stateid, &s->sc_stateid, 1);
+		if (ret) {
+			mutex_unlock(&stp->st_mutex);
+			goto out;
+		}
 		ret = nfserr_locks_held;
 		if (check_for_locks(stp->st_stid.sc_file,
-				    lockowner(stp->st_stateowner)))
-			break;
+				    lockowner(stp->st_stateowner))) {
+			mutex_unlock(&stp->st_mutex);
+			goto out;
+		}
+		spin_lock(&cl->cl_lock);
 		WARN_ON(!unhash_lock_stateid(stp));
 		spin_unlock(&cl->cl_lock);
+		mutex_unlock(&stp->st_mutex);
 		nfs4_put_stid(s);
 		ret = nfs_ok;
 		goto out;

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux