From patchwork Tue Oct 17 13:55:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Coddington X-Patchwork-Id: 10012129 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 5B20C601E7 for ; Tue, 17 Oct 2017 13:55:10 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 509B2288E7 for ; Tue, 17 Oct 2017 13:55:10 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 44D8C288EC; Tue, 17 Oct 2017 13:55:10 +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 8A747288E7 for ; Tue, 17 Oct 2017 13:55:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752622AbdJQNzI (ORCPT ); Tue, 17 Oct 2017 09:55:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59266 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751828AbdJQNzH (ORCPT ); Tue, 17 Oct 2017 09:55:07 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2CF0B4DD49; Tue, 17 Oct 2017 13:55:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2CF0B4DD49 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=bcodding@redhat.com Received: from bcodding.csb (ovpn-120-157.rdu2.redhat.com [10.10.120.157]) by smtp.corp.redhat.com (Postfix) with ESMTP id A634B1825A; Tue, 17 Oct 2017 13:55:06 +0000 (UTC) Received: by bcodding.csb (Postfix, from userid 24008) id 967BE10C3105; Tue, 17 Oct 2017 09:55:05 -0400 (EDT) From: Benjamin Coddington To: bfields@fieldses.org, Jeff Layton Cc: linux-nfs@vger.kernel.org Subject: [PATCH] nfsd4: Prevent the reuse of a closed stateid Date: Tue, 17 Oct 2017 09:55:05 -0400 Message-Id: <2087b4cab6c695a7df01c1135f1773c9b762f0b0.1508248427.git.bcodding@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 17 Oct 2017 13:55:07 +0000 (UTC) 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 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 --- 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);