From 80ce94a3902c9c4ef25c817e984cf406572ab279 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 18 Aug 2016 11:58:23 +0200 Subject: [PATCH] 4.4-stable patches added patches: nfs-don-t-create-zero-length-requests.patch nfsd-don-t-return-an-unhashed-lock-stateid-after-taking-mutex.patch nfsd-fix-race-between-free_stateid-and-lock.patch --- ...fs-don-t-create-zero-length-requests.patch | 44 +++++++ ...shed-lock-stateid-after-taking-mutex.patch | 88 ++++++++++++++ ...x-race-between-free_stateid-and-lock.patch | 111 ++++++++++++++++++ queue-4.4/series | 3 + 4 files changed, 246 insertions(+) create mode 100644 queue-4.4/nfs-don-t-create-zero-length-requests.patch create mode 100644 queue-4.4/nfsd-don-t-return-an-unhashed-lock-stateid-after-taking-mutex.patch create mode 100644 queue-4.4/nfsd-fix-race-between-free_stateid-and-lock.patch diff --git a/queue-4.4/nfs-don-t-create-zero-length-requests.patch b/queue-4.4/nfs-don-t-create-zero-length-requests.patch new file mode 100644 index 00000000000..1967562544a --- /dev/null +++ b/queue-4.4/nfs-don-t-create-zero-length-requests.patch @@ -0,0 +1,44 @@ +From 149a4fddd0a72d526abbeac0c8deaab03559836a Mon Sep 17 00:00:00 2001 +From: Benjamin Coddington +Date: Mon, 18 Jul 2016 10:41:57 -0400 +Subject: nfs: don't create zero-length requests + +From: Benjamin Coddington + +commit 149a4fddd0a72d526abbeac0c8deaab03559836a upstream. + +NFS doesn't expect requests with wb_bytes set to zero and may make +unexpected decisions about how to handle that request at the page IO layer. +Skip request creation if we won't have any wb_bytes in the request. + +Signed-off-by: Benjamin Coddington +Signed-off-by: Alexey Dobriyan +Reviewed-by: Weston Andros Adamson +Signed-off-by: Trond Myklebust +Signed-off-by: Greg Kroah-Hartman + +--- + fs/nfs/write.c | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +--- a/fs/nfs/write.c ++++ b/fs/nfs/write.c +@@ -1261,6 +1261,9 @@ int nfs_updatepage(struct file *file, st + dprintk("NFS: nfs_updatepage(%pD2 %d@%lld)\n", + file, count, (long long)(page_file_offset(page) + offset)); + ++ if (!count) ++ goto out; ++ + if (nfs_can_extend_write(file, page, inode)) { + count = max(count + offset, nfs_page_length(page)); + offset = 0; +@@ -1271,7 +1274,7 @@ int nfs_updatepage(struct file *file, st + nfs_set_pageerror(page); + else + __set_page_dirty_nobuffers(page); +- ++out: + dprintk("NFS: nfs_updatepage returns %d (isize %lld)\n", + status, (long long)i_size_read(inode)); + return status; diff --git a/queue-4.4/nfsd-don-t-return-an-unhashed-lock-stateid-after-taking-mutex.patch b/queue-4.4/nfsd-don-t-return-an-unhashed-lock-stateid-after-taking-mutex.patch new file mode 100644 index 00000000000..2468a3e9bb3 --- /dev/null +++ b/queue-4.4/nfsd-don-t-return-an-unhashed-lock-stateid-after-taking-mutex.patch @@ -0,0 +1,88 @@ +From dd257933fa4b9fea66a1195f8a15111029810abc Mon Sep 17 00:00:00 2001 +From: Jeff Layton +Date: Thu, 11 Aug 2016 10:37:39 -0400 +Subject: nfsd: don't return an unhashed lock stateid after taking mutex + +From: Jeff Layton + +commit dd257933fa4b9fea66a1195f8a15111029810abc upstream. + +nfsd4_lock will take the st_mutex before working with the stateid it +gets, but between the time when we drop the cl_lock and take the mutex, +the stateid could become unhashed (a'la FREE_STATEID). If that happens +the lock stateid returned to the client will be forgotten. + +Fix this by first moving the st_mutex acquisition into +lookup_or_create_lock_state. Then, have it check to see if the lock +stateid is still hashed after taking the mutex. If it's not, then put +the stateid and try the find/create again. + +Signed-off-by: Jeff Layton +Tested-by: Alexey Kodanev +Signed-off-by: J. Bruce Fields +Signed-off-by: Greg Kroah-Hartman + +--- + fs/nfsd/nfs4state.c | 25 ++++++++++++++++++++----- + 1 file changed, 20 insertions(+), 5 deletions(-) + +--- a/fs/nfsd/nfs4state.c ++++ b/fs/nfsd/nfs4state.c +@@ -5502,7 +5502,7 @@ static __be32 + lookup_or_create_lock_state(struct nfsd4_compound_state *cstate, + struct nfs4_ol_stateid *ost, + struct nfsd4_lock *lock, +- struct nfs4_ol_stateid **lst, bool *new) ++ struct nfs4_ol_stateid **plst, bool *new) + { + __be32 status; + struct nfs4_file *fi = ost->st_stid.sc_file; +@@ -5510,7 +5510,9 @@ lookup_or_create_lock_state(struct nfsd4 + struct nfs4_client *cl = oo->oo_owner.so_client; + struct inode *inode = d_inode(cstate->current_fh.fh_dentry); + struct nfs4_lockowner *lo; ++ struct nfs4_ol_stateid *lst; + unsigned int strhashval; ++ bool hashed; + + lo = find_lockowner_str(cl, &lock->lk_new_owner); + if (!lo) { +@@ -5526,12 +5528,27 @@ lookup_or_create_lock_state(struct nfsd4 + goto out; + } + +- *lst = find_or_create_lock_stateid(lo, fi, inode, ost, new); +- if (*lst == NULL) { ++retry: ++ lst = find_or_create_lock_stateid(lo, fi, inode, ost, new); ++ if (lst == NULL) { + status = nfserr_jukebox; + goto out; + } ++ ++ mutex_lock(&lst->st_mutex); ++ ++ /* See if it's still hashed to avoid race with FREE_STATEID */ ++ spin_lock(&cl->cl_lock); ++ hashed = !list_empty(&lst->st_perfile); ++ spin_unlock(&cl->cl_lock); ++ ++ if (!hashed) { ++ mutex_unlock(&lst->st_mutex); ++ nfs4_put_stid(&lst->st_stid); ++ goto retry; ++ } + status = nfs_ok; ++ *plst = lst; + out: + nfs4_put_stateowner(&lo->lo_owner); + return status; +@@ -5598,8 +5615,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struc + goto out; + status = lookup_or_create_lock_state(cstate, open_stp, lock, + &lock_stp, &new); +- if (status == nfs_ok) +- mutex_lock(&lock_stp->st_mutex); + } else { + status = nfs4_preprocess_seqid_op(cstate, + lock->lk_old_lock_seqid, diff --git a/queue-4.4/nfsd-fix-race-between-free_stateid-and-lock.patch b/queue-4.4/nfsd-fix-race-between-free_stateid-and-lock.patch new file mode 100644 index 00000000000..54f90bb95dc --- /dev/null +++ b/queue-4.4/nfsd-fix-race-between-free_stateid-and-lock.patch @@ -0,0 +1,111 @@ +From 42691398be08bd1fe99326911a0aa31f2c041d53 Mon Sep 17 00:00:00 2001 +From: Chuck Lever +Date: Thu, 11 Aug 2016 10:37:30 -0400 +Subject: nfsd: Fix race between FREE_STATEID and LOCK + +From: Chuck Lever + +commit 42691398be08bd1fe99326911a0aa31f2c041d53 upstream. + +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 +Fix-suggested-by: Jeff Layton +Signed-off-by: Chuck Lever +Tested-by: Alexey Kodanev +Signed-off-by: J. Bruce Fields +Signed-off-by: Greg Kroah-Hartman + +--- + fs/nfsd/nfs4state.c | 40 ++++++++++++++++++++++++++++------------ + 1 file changed, 28 insertions(+), 12 deletions(-) + +--- a/fs/nfsd/nfs4state.c ++++ b/fs/nfsd/nfs4state.c +@@ -4882,6 +4882,32 @@ nfsd4_test_stateid(struct svc_rqst *rqst + 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) +@@ -4889,7 +4915,6 @@ nfsd4_free_stateid(struct svc_rqst *rqst + 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; + +@@ -4908,18 +4933,9 @@ nfsd4_free_stateid(struct svc_rqst *rqst + 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); diff --git a/queue-4.4/series b/queue-4.4/series index 88c62ce7fca..1d3c7ef79db 100644 --- a/queue-4.4/series +++ b/queue-4.4/series @@ -59,3 +59,6 @@ mips-kvm-fix-mapped-fault-broken-commpage-handling.patch mips-kvm-add-missing-gfn-range-check.patch mips-kvm-fix-gfn-range-check-in-kseg0-tlb-faults.patch mips-kvm-propagate-kseg0-mapped-tlb-fault-errors.patch +nfs-don-t-create-zero-length-requests.patch +nfsd-fix-race-between-free_stateid-and-lock.patch +nfsd-don-t-return-an-unhashed-lock-stateid-after-taking-mutex.patch -- 2.47.3