From patchwork Mon Aug 8 18:59:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chuck Lever X-Patchwork-Id: 9269161 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 E39206075A for ; Mon, 8 Aug 2016 18:59:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CF33828159 for ; Mon, 8 Aug 2016 18:59:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C2E1628161; Mon, 8 Aug 2016 18:59:48 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,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 4EE7128159 for ; Mon, 8 Aug 2016 18:59:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752168AbcHHS7r (ORCPT ); Mon, 8 Aug 2016 14:59:47 -0400 Received: from mail-it0-f65.google.com ([209.85.214.65]:34326 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752167AbcHHS7q (ORCPT ); Mon, 8 Aug 2016 14:59:46 -0400 Received: by mail-it0-f65.google.com with SMTP id u186so6771691ita.1 for ; Mon, 08 Aug 2016 11:59:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:subject:from:to:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=YbMsyfQdHisLyO2I0SKQCWipgh1M/T9WCAX663gErCI=; b=sSNW97GUsuRr6rzXB916vF953fvG8NEIoWbX7ySMWMN6mwtMTxMLrQCFABkAnwQKE0 VnLcKYV0OYNZP1QMwJu6iWvdRIP7b/KD+Oi+ANvjrA9oX0kqAHeozWpqtoOhNa08f6KH dgVwYw6AOV0/Gk4cWKYUtRWNBHNexztb6oPgFyNcmH+pcMtBdmsORvuuf7OGuYxx7+Yl AeqG+wz71cHjqEMguI5e4iAHed48kz00HsFSrpbNNFF18qRf+LQ4mnWd9w9XvTby1gKB vefBecN41LZFcVeBcHQcY4DDzBjADMSwQJpUrUr489HBR1+X3LVCQGBpmJBasnfxrBtS CCFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:from:to:date:message-id :in-reply-to:references:user-agent:mime-version :content-transfer-encoding; bh=YbMsyfQdHisLyO2I0SKQCWipgh1M/T9WCAX663gErCI=; b=IShrS6QvWKgW5wl3MsfW6SzwIaRaNp0wlXGE1dJ0QiwgN+mWwuw8L46Ph9NAXCoSDn oktD69VLXhIUK7SATY/xYEnTO6Pppun3d3nZThg8YkB16ohbq3u3PlPOLJasBhtXE8MS +zaJiIq9nlKTmNY3eruQlEOANpTchkm5VzjxDio532hT9lpvGHSk6l9twweuwU7G9h0Z +plrzw2dd/mT0bZLtYPhPFOauylDjS1hmrKHvwlTY5Yy0Q6v7B5bIv+Y6JMJp1FHgnwy wgSSr+TWSxMF7vy6khv/rspJ5RxvnoAhpinH5eEyV7fr+WlB4iLBnQwFDb6ASgbB2O5O k1Gw== X-Gm-Message-State: AEkoouuZ1khx7/Y5IPQTFOqaAL4MtNkc9HHArJ+7S+x0ENDCV3XWNCIwZlH+SQhMStYGmQ== X-Received: by 10.36.55.146 with SMTP id r140mr20242719itr.73.1470682785886; Mon, 08 Aug 2016 11:59:45 -0700 (PDT) Received: from klimt.1015granger.net ([2604:8800:100:81fc:ec4:7aff:fe6c:6aa0]) by smtp.gmail.com with ESMTPSA id h70sm10737282ith.12.2016.08.08.11.59.44 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Aug 2016 11:59:45 -0700 (PDT) Subject: [PATCH v3 1/2] nfsd: Fix race between FREE_STATEID and LOCK From: Chuck Lever To: linux-nfs@vger.kernel.org Date: Mon, 08 Aug 2016 14:59:44 -0400 Message-ID: <20160808185944.11661.30644.stgit@klimt.1015granger.net> In-Reply-To: <20160808184711.11661.86427.stgit@klimt.1015granger.net> References: <20160808184711.11661.86427.stgit@klimt.1015granger.net> User-Agent: StGit/0.17.1-dirty 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 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 --- fs/nfsd/nfs4state.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) -- 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 b921123..de868fe 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4885,6 +4885,32 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, 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) @@ -4892,7 +4918,6 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, 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; @@ -4911,18 +4936,9 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, 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);