While running generic/089 on NFSv4.1, the following on-the-wire
exchange
occurs:
Client Server
---------- ----------
OPEN (owner A) ->
<- OPEN response: state A1
CLOSE (state A1)->
OPEN (owner A) ->
<- CLOSE response: state A2
<- OPEN response: state A3
LOCK (state A3) ->
<- LOCK response: NFS4_BAD_STATEID
The server should not be returning state A3 in response to the second
OPEN
after processing the CLOSE and sending back state A2. The problem is
that
nfsd4_process_open2() is able to find an existing open state just
after
nfsd4_close() has incremented the state's sequence number but before
calling unhash_ol_stateid().
Fix this by using the state's sc_type field to identify closed state,
and
protect the update of that field with st_mutex.
nfsd4_find_existing_open()
will return with the st_mutex held, so that the state will not
transition
between being looked up and then updated in nfsd4_process_open2().
Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
---
fs/nfsd/nfs4state.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0c04f81aa63b..17473a092d01 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3503,11 +3503,12 @@ static const struct
nfs4_stateowner_operations openowner_ops = {
static struct nfs4_ol_stateid *
nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open
*open)
{
- struct nfs4_ol_stateid *local, *ret = NULL;
+ struct nfs4_ol_stateid *local, *ret;
struct nfs4_openowner *oo = open->op_openowner;
- lockdep_assert_held(&fp->fi_lock);
-
+retry:
+ ret = NULL;
+ spin_lock(&fp->fi_lock);
list_for_each_entry(local, &fp->fi_stateids, st_perfile) {
/* ignore lock owners */
if (local->st_stateowner->so_is_open_owner == 0)
@@ -3518,6 +3519,18 @@ nfsd4_find_existing_open(struct nfs4_file *fp,
struct nfsd4_open *open)
break;
}
}
+ spin_unlock(&fp->fi_lock);
+
+ /* Did we race with CLOSE? */
+ if (ret) {
+ mutex_lock(&ret->st_mutex);
+ if (ret->st_stid.sc_type != NFS4_OPEN_STID) {
+ mutex_unlock(&ret->st_mutex);
+ nfs4_put_stid(&ret->st_stid);
+ goto retry;
+ }
+ }
+
return ret;
}
@@ -3566,7 +3579,6 @@ init_open_stateid(struct nfs4_file *fp, struct
nfsd4_open *open)
mutex_lock(&stp->st_mutex);
spin_lock(&oo->oo_owner.so_client->cl_lock);
- spin_lock(&fp->fi_lock);
retstp = nfsd4_find_existing_open(fp, open);
if (retstp)
@@ -3583,13 +3595,13 @@ init_open_stateid(struct nfs4_file *fp,
struct nfsd4_open *open)
stp->st_deny_bmap = 0;
stp->st_openstp = NULL;
list_add(&stp->st_perstateowner, &oo->oo_owner.so_stateids);
+ spin_lock(&fp->fi_lock);
list_add(&stp->st_perfile, &fp->fi_stateids);
+ spin_unlock(&fp->fi_lock);
out_unlock:
- spin_unlock(&fp->fi_lock);
spin_unlock(&oo->oo_owner.so_client->cl_lock);
if (retstp) {
- mutex_lock(&retstp->st_mutex);
/* To keep mutex tracking happy */
mutex_unlock(&stp->st_mutex);
stp = retstp;
@@ -4403,9 +4415,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp,
struct svc_fh *current_fh, struct nf
status = nfs4_check_deleg(cl, open, &dp);
if (status)
goto out;
- spin_lock(&fp->fi_lock);
stp = nfsd4_find_existing_open(fp, open);
- spin_unlock(&fp->fi_lock);
} else {
open->op_file = NULL;
status = nfserr_bad_stateid;
@@ -4419,7 +4429,6 @@ nfsd4_process_open2(struct svc_rqst *rqstp,
struct svc_fh *current_fh, struct nf
*/
if (stp) {
/* Stateid was found, this is an OPEN upgrade */
- mutex_lock(&stp->st_mutex);
status = nfs4_upgrade_open(rqstp, fp, current_fh, stp, open);
if (status) {
mutex_unlock(&stp->st_mutex);
@@ -5294,7 +5303,6 @@ static void nfsd4_close_open_stateid(struct
nfs4_ol_stateid *s)
bool unhashed;
LIST_HEAD(reaplist);
- s->st_stid.sc_type = NFS4_CLOSED_STID;
spin_lock(&clp->cl_lock);
unhashed = unhash_open_stateid(s, &reaplist);
@@ -5335,6 +5343,7 @@ nfsd4_close(struct svc_rqst *rqstp, struct
nfsd4_compound_state *cstate,
if (status)
goto out;
nfs4_inc_and_copy_stateid(&close->cl_stateid, &stp->st_stid);
+ stp->st_stid.sc_type = NFS4_CLOSED_STID;
mutex_unlock(&stp->st_mutex);
nfsd4_close_open_stateid(stp);
--
2.9.3
