From patchwork Tue Oct 31 00:09:47 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trond Myklebust X-Patchwork-Id: 10033485 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 2E0A16039A for ; Tue, 31 Oct 2017 00:10:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1FDB928945 for ; Tue, 31 Oct 2017 00:10:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 148AA28980; Tue, 31 Oct 2017 00:10: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.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID 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 8D2A128945 for ; Tue, 31 Oct 2017 00:09:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753399AbdJaAJ6 (ORCPT ); Mon, 30 Oct 2017 20:09:58 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:54435 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753186AbdJaAJ5 (ORCPT ); Mon, 30 Oct 2017 20:09:57 -0400 Received: by mail-io0-f194.google.com with SMTP id e89so31184830ioi.11 for ; Mon, 30 Oct 2017 17:09:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=/ONWQOjcck0OXJbH1MAHLqElJFymMKhTSGuC6mJ25gI=; b=l6piU6Acn2zb+fb+vHiSKUhrhd/scj5+vQtIph7S2JEl94RUHa+KJuZJQjm1g/1D3n O7t8ewvgSJNjWNaqArs00V6r8gj1CQxa3S6pIdfiVQsTlnRcJXKEhTSXc/kzQGYcfqfj yLByoYh5SdmWo50A2TX3s8Wf/VS/BjWzOx3+cHaaNZlAxlI+BSEguFdnDuaptYVIUGYa xFvPVjVS8cuAUzcY+z2Ll1Uen3AG1mUloRvhoP/Bf+gSGiKhDky3N7JLIH8jvoYNiIZk 6jNbNXhO6t4FHqQYkx62b3PAc0yIQaagHZWDangovs7xqTyQCuKJbyyCdkncsD9UWPVq sb7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references:mime-version:content-transfer-encoding; bh=/ONWQOjcck0OXJbH1MAHLqElJFymMKhTSGuC6mJ25gI=; b=GBdiELRr8XmZDQdfs2908e3pIS0QL2n1FLk3ie+tieF3ySRl6uDrNlDJWZX0nabC3j 6gda979abCXRQVW95K5AP/KYihGdw02RB0T44VkeIU5mBZnSlBniY9W5FVRA6o7sFvcP LEK7SuJ08SaYqJrGdruIPKeZXcU2O+sbSl0/DQI4rPRLMJni9xqcL6F/3NilQ2l+tQxT HRSbOTP6yRKKyfYqJy30XDQpxvF0WHYmVekofVnEm9ZFqAsAY1BmaE0DizRG4omPEnk1 TAAQqxxBYdjS31CgzDK7BwGOSla75h65qDEY6GVcH/GQNymSxMIK8WgCFuxuVvnNuAvF pNPQ== X-Gm-Message-State: AMCzsaWwKFwoqFjkmrvSiccl8HN9h0EOgAmABZxIFYwh+XutCk5Ga1Mw yKCTdfb2Y4nhJWrZExNeVQ== X-Google-Smtp-Source: ABhQp+QXSibjkc+QSGvswpyeYQOFSZjQFGQo9SW5MO9TLjYk9ZWrC3xFqE2rh/XpbcWcaUa5rbG0lg== X-Received: by 10.36.105.65 with SMTP id e62mr712601itc.16.1509408596423; Mon, 30 Oct 2017 17:09:56 -0700 (PDT) Received: from localhost.localdomain (c-68-49-162-121.hsd1.mi.comcast.net. [68.49.162.121]) by smtp.gmail.com with ESMTPSA id h97sm63653iod.87.2017.10.30.17.09.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 30 Oct 2017 17:09:55 -0700 (PDT) From: Trond Myklebust To: bfields@fieldses.org Cc: linux-nfs@vger.kernel.org Subject: [PATCH 1/5] nfsd: Fix stateid races between OPEN and CLOSE Date: Mon, 30 Oct 2017 20:09:47 -0400 Message-Id: <20171031000951.18294-2-trond.myklebust@primarydata.com> X-Mailer: git-send-email 2.13.6 In-Reply-To: <20171031000951.18294-1-trond.myklebust@primarydata.com> References: <20171031000951.18294-1-trond.myklebust@primarydata.com> MIME-Version: 1.0 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 Open file stateids can linger on the nfs4_file list of stateids even after they have been closed. In order to avoid reusing such a stateid, and confusing the client, we need to recheck the nfs4_stid's type after taking the mutex. Otherwise, we risk reusing an old stateid that was already closed, which will confuse clients that expect new stateids to conform to RFC7530 Sections 9.1.4.2 and 16.2.5 or RFC5661 Sections 8.2.2 and 18.2.4. Signed-off-by: Trond Myklebust Cc: stable@vger.kernel.org --- fs/nfsd/nfs4state.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 8 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 0c04f81aa63b..b246aaa20863 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -3512,7 +3512,9 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open) /* ignore lock owners */ if (local->st_stateowner->so_is_open_owner == 0) continue; - if (local->st_stateowner == &oo->oo_owner) { + if (local->st_stateowner != &oo->oo_owner) + continue; + if (local->st_stid.sc_type == NFS4_OPEN_STID) { ret = local; atomic_inc(&ret->st_stid.sc_count); break; @@ -3521,6 +3523,52 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open) return ret; } +static __be32 +nfsd4_verify_open_stid(struct nfs4_stid *s) +{ + __be32 ret = nfs_ok; + + switch (s->sc_type) { + default: + break; + case NFS4_CLOSED_STID: + case NFS4_CLOSED_DELEG_STID: + ret = nfserr_bad_stateid; + break; + case NFS4_REVOKED_DELEG_STID: + ret = nfserr_deleg_revoked; + } + return ret; +} + +/* Lock the stateid st_mutex, and deal with races with CLOSE */ +static __be32 +nfsd4_lock_ol_stateid(struct nfs4_ol_stateid *stp) +{ + __be32 ret; + + mutex_lock(&stp->st_mutex); + ret = nfsd4_verify_open_stid(&stp->st_stid); + if (ret != nfs_ok) + mutex_unlock(&stp->st_mutex); + return ret; +} + +static struct nfs4_ol_stateid * +nfsd4_find_and_lock_existing_open(struct nfs4_file *fp, struct nfsd4_open *open) +{ + struct nfs4_ol_stateid *stp; + for (;;) { + spin_lock(&fp->fi_lock); + stp = nfsd4_find_existing_open(fp, open); + spin_unlock(&fp->fi_lock); + if (!stp || nfsd4_lock_ol_stateid(stp) == nfs_ok) + break; + nfs4_put_stid(&stp->st_stid); + } + return stp; +} + static struct nfs4_openowner * alloc_init_open_stateowner(unsigned int strhashval, struct nfsd4_open *open, struct nfsd4_compound_state *cstate) @@ -3565,6 +3613,7 @@ init_open_stateid(struct nfs4_file *fp, struct nfsd4_open *open) mutex_init(&stp->st_mutex); mutex_lock(&stp->st_mutex); +retry: spin_lock(&oo->oo_owner.so_client->cl_lock); spin_lock(&fp->fi_lock); @@ -3589,7 +3638,11 @@ init_open_stateid(struct nfs4_file *fp, struct nfsd4_open *open) spin_unlock(&fp->fi_lock); spin_unlock(&oo->oo_owner.so_client->cl_lock); if (retstp) { - mutex_lock(&retstp->st_mutex); + /* Handle races with CLOSE */ + if (nfsd4_lock_ol_stateid(retstp) != nfs_ok) { + nfs4_put_stid(&retstp->st_stid); + goto retry; + } /* To keep mutex tracking happy */ mutex_unlock(&stp->st_mutex); stp = retstp; @@ -4403,9 +4456,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); + stp = nfsd4_find_and_lock_existing_open(fp, open); } else { open->op_file = NULL; status = nfserr_bad_stateid; @@ -4419,7 +4470,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 +5344,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); @@ -5334,10 +5383,12 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, nfsd4_bump_seqid(cstate, status); if (status) goto out; + + stp->st_stid.sc_type = NFS4_CLOSED_STID; nfs4_inc_and_copy_stateid(&close->cl_stateid, &stp->st_stid); - mutex_unlock(&stp->st_mutex); nfsd4_close_open_stateid(stp); + mutex_unlock(&stp->st_mutex); /* put reference from nfs4_preprocess_seqid_op */ nfs4_put_stid(&stp->st_stid);