Skip to content

Commit feb9dad

Browse files
verygreenJ. Bruce Fields
verygreen
authored and
J. Bruce Fields
committed
nfsd: Always lock state exclusively.
It used to be the case that state had an rwlock that was locked for write by downgrades, but for read for upgrades (opens). Well, the problem is if there are two competing opens for the same state, they step on each other toes potentially leading to leaking file descriptors from the state structure, since access mode is a bitmap only set once. Signed-off-by: Oleg Drokin <green@linuxhacker.ru> Cc: stable@vger.kernel.org Signed-off-by: J. Bruce Fields <bfields@redhat.com>
1 parent 39a9bea commit feb9dad

File tree

2 files changed

+21
-21
lines changed

2 files changed

+21
-21
lines changed

‎fs/nfsd/nfs4state.c

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3502,7 +3502,7 @@ init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_file *fp,
35023502
stp->st_access_bmap = 0;
35033503
stp->st_deny_bmap = 0;
35043504
stp->st_openstp = NULL;
3505-
init_rwsem(&stp->st_rwsem);
3505+
mutex_init(&stp->st_mutex);
35063506
list_add(&stp->st_perstateowner, &oo->oo_owner.so_stateids);
35073507
list_add(&stp->st_perfile, &fp->fi_stateids);
35083508

@@ -4335,10 +4335,10 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
43354335
*/
43364336
if (stp) {
43374337
/* Stateid was found, this is an OPEN upgrade */
4338-
down_read(&stp->st_rwsem);
4338+
mutex_lock(&stp->st_mutex);
43394339
status = nfs4_upgrade_open(rqstp, fp, current_fh, stp, open);
43404340
if (status) {
4341-
up_read(&stp->st_rwsem);
4341+
mutex_unlock(&stp->st_mutex);
43424342
goto out;
43434343
}
43444344
} else {
@@ -4348,19 +4348,19 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
43484348
if (swapstp) {
43494349
nfs4_put_stid(&stp->st_stid);
43504350
stp = swapstp;
4351-
down_read(&stp->st_rwsem);
4351+
mutex_lock(&stp->st_mutex);
43524352
status = nfs4_upgrade_open(rqstp, fp, current_fh,
43534353
stp, open);
43544354
if (status) {
4355-
up_read(&stp->st_rwsem);
4355+
mutex_unlock(&stp->st_mutex);
43564356
goto out;
43574357
}
43584358
goto upgrade_out;
43594359
}
4360-
down_read(&stp->st_rwsem);
4360+
mutex_lock(&stp->st_mutex);
43614361
status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open);
43624362
if (status) {
4363-
up_read(&stp->st_rwsem);
4363+
mutex_unlock(&stp->st_mutex);
43644364
release_open_stateid(stp);
43654365
goto out;
43664366
}
@@ -4372,7 +4372,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
43724372
}
43734373
upgrade_out:
43744374
nfs4_inc_and_copy_stateid(&open->op_stateid, &stp->st_stid);
4375-
up_read(&stp->st_rwsem);
4375+
mutex_unlock(&stp->st_mutex);
43764376

43774377
if (nfsd4_has_session(&resp->cstate)) {
43784378
if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
@@ -4977,12 +4977,12 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_
49774977
* revoked delegations are kept only for free_stateid.
49784978
*/
49794979
return nfserr_bad_stateid;
4980-
down_write(&stp->st_rwsem);
4980+
mutex_lock(&stp->st_mutex);
49814981
status = check_stateid_generation(stateid, &stp->st_stid.sc_stateid, nfsd4_has_session(cstate));
49824982
if (status == nfs_ok)
49834983
status = nfs4_check_fh(current_fh, &stp->st_stid);
49844984
if (status != nfs_ok)
4985-
up_write(&stp->st_rwsem);
4985+
mutex_unlock(&stp->st_mutex);
49864986
return status;
49874987
}
49884988

@@ -5030,7 +5030,7 @@ static __be32 nfs4_preprocess_confirmed_seqid_op(struct nfsd4_compound_state *cs
50305030
return status;
50315031
oo = openowner(stp->st_stateowner);
50325032
if (!(oo->oo_flags & NFS4_OO_CONFIRMED)) {
5033-
up_write(&stp->st_rwsem);
5033+
mutex_unlock(&stp->st_mutex);
50345034
nfs4_put_stid(&stp->st_stid);
50355035
return nfserr_bad_stateid;
50365036
}
@@ -5062,12 +5062,12 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
50625062
oo = openowner(stp->st_stateowner);
50635063
status = nfserr_bad_stateid;
50645064
if (oo->oo_flags & NFS4_OO_CONFIRMED) {
5065-
up_write(&stp->st_rwsem);
5065+
mutex_unlock(&stp->st_mutex);
50665066
goto put_stateid;
50675067
}
50685068
oo->oo_flags |= NFS4_OO_CONFIRMED;
50695069
nfs4_inc_and_copy_stateid(&oc->oc_resp_stateid, &stp->st_stid);
5070-
up_write(&stp->st_rwsem);
5070+
mutex_unlock(&stp->st_mutex);
50715071
dprintk("NFSD: %s: success, seqid=%d stateid=" STATEID_FMT "\n",
50725072
__func__, oc->oc_seqid, STATEID_VAL(&stp->st_stid.sc_stateid));
50735073

@@ -5143,7 +5143,7 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,
51435143
nfs4_inc_and_copy_stateid(&od->od_stateid, &stp->st_stid);
51445144
status = nfs_ok;
51455145
put_stateid:
5146-
up_write(&stp->st_rwsem);
5146+
mutex_unlock(&stp->st_mutex);
51475147
nfs4_put_stid(&stp->st_stid);
51485148
out:
51495149
nfsd4_bump_seqid(cstate, status);
@@ -5196,7 +5196,7 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
51965196
if (status)
51975197
goto out;
51985198
nfs4_inc_and_copy_stateid(&close->cl_stateid, &stp->st_stid);
5199-
up_write(&stp->st_rwsem);
5199+
mutex_unlock(&stp->st_mutex);
52005200

52015201
nfsd4_close_open_stateid(stp);
52025202

@@ -5422,7 +5422,7 @@ init_lock_stateid(struct nfs4_ol_stateid *stp, struct nfs4_lockowner *lo,
54225422
stp->st_access_bmap = 0;
54235423
stp->st_deny_bmap = open_stp->st_deny_bmap;
54245424
stp->st_openstp = open_stp;
5425-
init_rwsem(&stp->st_rwsem);
5425+
mutex_init(&stp->st_mutex);
54265426
list_add(&stp->st_locks, &open_stp->st_locks);
54275427
list_add(&stp->st_perstateowner, &lo->lo_owner.so_stateids);
54285428
spin_lock(&fp->fi_lock);
@@ -5591,7 +5591,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
55915591
&open_stp, nn);
55925592
if (status)
55935593
goto out;
5594-
up_write(&open_stp->st_rwsem);
5594+
mutex_unlock(&open_stp->st_mutex);
55955595
open_sop = openowner(open_stp->st_stateowner);
55965596
status = nfserr_bad_stateid;
55975597
if (!same_clid(&open_sop->oo_owner.so_client->cl_clientid,
@@ -5600,7 +5600,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
56005600
status = lookup_or_create_lock_state(cstate, open_stp, lock,
56015601
&lock_stp, &new);
56025602
if (status == nfs_ok)
5603-
down_write(&lock_stp->st_rwsem);
5603+
mutex_lock(&lock_stp->st_mutex);
56045604
} else {
56055605
status = nfs4_preprocess_seqid_op(cstate,
56065606
lock->lk_old_lock_seqid,
@@ -5704,7 +5704,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
57045704
seqid_mutating_err(ntohl(status)))
57055705
lock_sop->lo_owner.so_seqid++;
57065706

5707-
up_write(&lock_stp->st_rwsem);
5707+
mutex_unlock(&lock_stp->st_mutex);
57085708

57095709
/*
57105710
* If this is a new, never-before-used stateid, and we are
@@ -5874,7 +5874,7 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
58745874
fput:
58755875
fput(filp);
58765876
put_stateid:
5877-
up_write(&stp->st_rwsem);
5877+
mutex_unlock(&stp->st_mutex);
58785878
nfs4_put_stid(&stp->st_stid);
58795879
out:
58805880
nfsd4_bump_seqid(cstate, status);

‎fs/nfsd/state.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ struct nfs4_ol_stateid {
535535
unsigned char st_access_bmap;
536536
unsigned char st_deny_bmap;
537537
struct nfs4_ol_stateid *st_openstp;
538-
struct rw_semaphore st_rwsem;
538+
struct mutex st_mutex;
539539
};
540540

541541
static inline struct nfs4_ol_stateid *openlockstateid(struct nfs4_stid *s)

0 commit comments

Comments
 (0)