nfsd: Fix race between FREE_STATEID and LOCK
When running LTP's nfslock01 test, the Linux client can send a LOCK and a FREE_STATEID request at the same time. The outcome is: Frame 324 R OPEN stateid [2,O] Frame 115004 C LOCK lockowner_is_new stateid [2,O] offset 672000 len 64 Frame 115008 R LOCK stateid [1,L] Frame 115012 C WRITE stateid [0,L] offset 672000 len 64 Frame 115016 R WRITE NFS4_OK Frame 115019 C LOCKU stateid [1,L] offset 672000 len 64 Frame 115022 R LOCKU NFS4_OK Frame 115025 C FREE_STATEID stateid [2,L] Frame 115026 C LOCK lockowner_is_new stateid [2,O] offset 672128 len 64 Frame 115029 R FREE_STATEID NFS4_OK Frame 115030 R LOCK stateid [3,L] Frame 115034 C WRITE stateid [0,L] offset 672128 len 64 Frame 115038 R WRITE NFS4ERR_BAD_STATEID In other words, the server returns stateid L in a successful LOCK reply, but it has already released it. Subsequent uses of stateid L fail. To address this, protect the generation check 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@oracle.com> Fix-suggested-by: Jeff Layton <jlayton@redhat.com> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Tested-by: Alexey Kodanev <alexey.kodanev@oracle.com> Cc: stable@vger.kernel.org Signed-off-by: J. Bruce Fields <bfields@redhat.com>
This commit is contained in:
		
							parent
							
								
									502aa0a5be
								
							
						
					
					
						commit
						42691398be
					
				| @ -4903,6 +4903,32 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, | ||||
| 	return nfs_ok; | ||||
| } | ||||
| 
 | ||||
| static __be32 | ||||
| nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stid *s) | ||||
| { | ||||
| 	struct nfs4_ol_stateid *stp = openlockstateid(s); | ||||
| 	__be32 ret; | ||||
| 
 | ||||
| 	mutex_lock(&stp->st_mutex); | ||||
| 
 | ||||
| 	ret = check_stateid_generation(stateid, &s->sc_stateid, 1); | ||||
| 	if (ret) | ||||
| 		goto out; | ||||
| 
 | ||||
| 	ret = nfserr_locks_held; | ||||
| 	if (check_for_locks(stp->st_stid.sc_file, | ||||
| 			    lockowner(stp->st_stateowner))) | ||||
| 		goto out; | ||||
| 
 | ||||
| 	release_lock_stateid(stp); | ||||
| 	ret = nfs_ok; | ||||
| 
 | ||||
| out: | ||||
| 	mutex_unlock(&stp->st_mutex); | ||||
| 	nfs4_put_stid(s); | ||||
| 	return ret; | ||||
| } | ||||
| 
 | ||||
| __be32 | ||||
| nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, | ||||
| 		   struct nfsd4_free_stateid *free_stateid) | ||||
| @ -4910,7 +4936,6 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, | ||||
| 	stateid_t *stateid = &free_stateid->fr_stateid; | ||||
| 	struct nfs4_stid *s; | ||||
| 	struct nfs4_delegation *dp; | ||||
| 	struct nfs4_ol_stateid *stp; | ||||
| 	struct nfs4_client *cl = cstate->session->se_client; | ||||
| 	__be32 ret = nfserr_bad_stateid; | ||||
| 
 | ||||
| @ -4929,18 +4954,9 @@ 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; | ||||
| 		stp = openlockstateid(s); | ||||
| 		ret = nfserr_locks_held; | ||||
| 		if (check_for_locks(stp->st_stid.sc_file, | ||||
| 				    lockowner(stp->st_stateowner))) | ||||
| 			break; | ||||
| 		WARN_ON(!unhash_lock_stateid(stp)); | ||||
| 		atomic_inc(&s->sc_count); | ||||
| 		spin_unlock(&cl->cl_lock); | ||||
| 		nfs4_put_stid(s); | ||||
| 		ret = nfs_ok; | ||||
| 		ret = nfsd4_free_lock_stateid(stateid, s); | ||||
| 		goto out; | ||||
| 	case NFS4_REVOKED_DELEG_STID: | ||||
| 		dp = delegstateid(s); | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user