From patchwork Fri Nov 3 12:00:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Trond Myklebust X-Patchwork-Id: 10039915 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 5785360384 for ; Fri, 3 Nov 2017 12:00:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3CA02287D0 for ; Fri, 3 Nov 2017 12:00:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 31A07295B9; Fri, 3 Nov 2017 12:00:27 +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 B6018289C5 for ; Fri, 3 Nov 2017 12:00:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756149AbdKCMAZ (ORCPT ); Fri, 3 Nov 2017 08:00:25 -0400 Received: from mail-io0-f195.google.com ([209.85.223.195]:54490 "EHLO mail-io0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756145AbdKCMAV (ORCPT ); Fri, 3 Nov 2017 08:00:21 -0400 Received: by mail-io0-f195.google.com with SMTP id e89so5663355ioi.11 for ; Fri, 03 Nov 2017 05:00:21 -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=HvFjjCXilORH3BQ7e9IiKXdNaMAOKJocn44TqILkrY1FaWN1x3v5i+WVu5NAB8SEhy NaM95WLg6vNpQL1vcf0zBPhkvaLGau7fLF4MlVIv7HGaO1qsuIAqP7mAKkrhIFHh7FHM UM0xFYyrSegsG24NuHeQ+UDtVTwjK5mB2HQmfm7CttjfRFAHj8QDS+L2dLpPVXO5QYs8 ePsx2is6sBjYP5HHYo7yQZHrVi/gSPnnJWDkcGKYT5vqbhuli+4s/0nC1gytbRJlxx1T TKfVvJ6xhyWQMm5gxbY2xuB1A0uya25Se3VnKyVvBuPGiioOl94RF0z2jh9dB5SF6gGH Zj6g== 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=liZUH0HVIbdgx8BvBYn4v+6Qc7YaawMl4vbuaSt5lWv1R/vEYmNLwrdqSgRjRp01fT wbnowre+qy10JgQGY0b0J9slcFvfPh2XrLEv2+Ncgh6pDCLHCDTpFBFsHh3pa0KUb47o yfSHvOk6FQ90h3QD4/DSU6jEiQ6xnxE8trPNzFnOvDY/C1I2SKRjYda6UG+JqhK7RkUW yExIl7WQFtsQR4DlN/Z/SCM42VCms673cfFwcCZ8dy4KATPxPXG9OcZcl1Nw8bLJQ7uV d4KURyQYckkQB1gBpYIm2CkB+Cq7LSnXvIR/c8nLJ4wtratt2Md3vTrhjPW1L8ofFcHH Stug== X-Gm-Message-State: AJaThX4EFd5yyQnqFZJOl3tHW2FCyCQjkfQqOaNzgHMVbGSdMo0QFC+z 6oXvFmJ3QHXqlcjBaSh1Gg== X-Google-Smtp-Source: ABhQp+R5r8esMwQHGe59fRpJq6UuhlvuHgxwlmdcdAGIVRJUsfGtVjrDEqCOjAgfa2t5WbIAhE1YRA== X-Received: by 10.107.176.14 with SMTP id z14mr8372922ioe.50.1509710420684; Fri, 03 Nov 2017 05:00:20 -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 k18sm2620923ioc.75.2017.11.03.05.00.19 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 03 Nov 2017 05:00:19 -0700 (PDT) From: Trond Myklebust To: bfields@fieldses.org Cc: linux-nfs@vger.kernel.org Subject: [PATCH v2 1/7] nfsd: Fix stateid races between OPEN and CLOSE Date: Fri, 3 Nov 2017 08:00:10 -0400 Message-Id: <20171103120016.6200-2-trond.myklebust@primarydata.com> X-Mailer: git-send-email 2.13.6 In-Reply-To: <20171103120016.6200-1-trond.myklebust@primarydata.com> References: <20171103120016.6200-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);