From patchwork Wed Jan 17 21:30:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "J. Bruce Fields" X-Patchwork-Id: 10172007 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E4A0A603ED for ; Wed, 17 Jan 2018 21:31:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D49812521E for ; Wed, 17 Jan 2018 21:31:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A5316269E2; Wed, 17 Jan 2018 21:31:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B5F3526D05 for ; Wed, 17 Jan 2018 21:30:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753433AbeAQVa6 (ORCPT ); Wed, 17 Jan 2018 16:30:58 -0500 Received: from fieldses.org ([173.255.197.46]:53314 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753302AbeAQVa5 (ORCPT ); Wed, 17 Jan 2018 16:30:57 -0500 Received: by fieldses.org (Postfix, from userid 2815) id 8AD452407; Wed, 17 Jan 2018 16:30:57 -0500 (EST) Date: Wed, 17 Jan 2018 16:30:57 -0500 To: Chuck Lever Cc: Trond Myklebust , Bruce Fields , Linux NFS Mailing List Subject: Re: [PATCH] nfsd: Detect unhashed stids in nfsd4_verify_open_stid() Message-ID: <20180117213057.GA1441@fieldses.org> References: <20180112224230.129322-1-trond.myklebust@primarydata.com> <3D728866-A30E-420D-9001-F4B212432C36@oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3D728866-A30E-420D-9001-F4B212432C36@oracle.com> User-Agent: Mutt/1.5.21 (2010-09-15) From: bfields@fieldses.org (J. Bruce Fields) Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Jan 12, 2018 at 09:15:46PM -0500, Chuck Lever wrote: > > > > On Jan 12, 2018, at 5:42 PM, Trond Myklebust wrote: > > > > The state of the stid is guaranteed by 2 locks: > > - The nfs4_client 'cl_lock' spinlock > > - The nfs4_ol_stateid 'st_mutex' mutex > > > > so it is quite possible for the stid to be unhashed after lookup, > > but before calling nfsd4_lock_ol_stateid(). So we do need to check > > for a zero value for 'sc_type' in nfsd4_verify_open_stid(). > > > > Signed-off-by: Trond Myklebust > > Three successful passes of the git regression suite on NFSv4.1 > Three successful passes of xfstests on NFSv4.1 > > Tested-by: Chuck Lever Thanks! Applying. Then I think this makes a couple of "sc_type = NFS4_CLOSED_STID"'s superfluous. --b. commit 17693d95b3d3 Author: J. Bruce Fields Date: Wed Jan 17 16:25:59 2018 -0500 nfsd4: don't set lock stateid's sc_type to CLOSED There's no point I can see to stp->st_stid.sc_type = NFS4_CLOSED_STID; given release_lock_stateid immediately sets sc_type to 0. That set of sc_type to 0 should be enough to prevent it being used where we don't want it to be; NFS4_CLOSED_STID should only be needed for actual open stateid's that are actually closed. Signed-off-by: J. Bruce Fields --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 5a75135f5f53..150521c9671b 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -5183,7 +5183,6 @@ nfsd4_free_lock_stateid(stateid_t *stateid, struct nfs4_stid *s) lockowner(stp->st_stateowner))) goto out; - stp->st_stid.sc_type = NFS4_CLOSED_STID; release_lock_stateid(stp); ret = nfs_ok; @@ -6079,10 +6078,8 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, * If this is a new, never-before-used stateid, and we are * returning an error, then just go ahead and release it. */ - if (status && new) { - lock_stp->st_stid.sc_type = NFS4_CLOSED_STID; + if (status && new) release_lock_stateid(lock_stp); - } mutex_unlock(&lock_stp->st_mutex);