From patchwork Thu Jun 9 21:01:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oleg Drokin X-Patchwork-Id: 9167897 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 0AC8B60467 for ; Thu, 9 Jun 2016 21:02:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F195E28342 for ; Thu, 9 Jun 2016 21:02:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E613A28355; Thu, 9 Jun 2016 21:02:01 +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 F382728342 for ; Thu, 9 Jun 2016 21:02:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750792AbcFIVB7 (ORCPT ); Thu, 9 Jun 2016 17:01:59 -0400 Received: from linuxhacker.ru ([217.76.32.60]:36158 "EHLO fiona.linuxhacker.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750756AbcFIVB6 (ORCPT ); Thu, 9 Jun 2016 17:01:58 -0400 Received: from intelbox2.localnet (c-73-190-129-164.hsd1.tn.comcast.net [73.190.129.164]) (authenticated bits=0) by fiona.linuxhacker.ru (8.15.2/8.14.7) with ESMTPSA id u59L1gGv3920348 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 10 Jun 2016 00:01:44 +0300 From: Oleg Drokin To: Jeff Layton , "J . Bruce Fields" Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, Oleg Drokin Subject: [PATCH] nfsd: Close a race between access checking/setting in nfs4_get_vfs_file Date: Thu, 9 Jun 2016 17:01:39 -0400 Message-Id: <1465506099-475103-1-git-send-email-green@linuxhacker.ru> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1465406560.30890.10.camel@poochiereds.net> References: <1465406560.30890.10.camel@poochiereds.net> 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 Currently there's an unprotected access mode check in nfs4_upgrade_open that then calls nfs4_get_vfs_file which in turn assumes whatever access mode was present in the state is still valid which is racy. Two nfs4_get_vfs_file van enter the same path as result and get two references to nfs4_file, but later drop would only happens once because access mode is only denoted by bits, so no refcounting. The locking around access mode testing is introduced to avoid this race. Signed-off-by: Oleg Drokin --- This patch performs equally well to the st_rwsem -> mutex conversion, but is a bit ligher-weight I imagine. For one it seems to allow truncates in parallel if we ever want it. fs/nfsd/nfs4state.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index f5f82e1..d4b9eba 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -3958,6 +3958,11 @@ static __be32 nfs4_get_vfs_file(struct svc_rqst *rqstp, struct nfs4_file *fp, spin_lock(&fp->fi_lock); + if (test_access(open->op_share_access, stp)) { + spin_unlock(&fp->fi_lock); + return nfserr_eagain; + } + /* * Are we trying to set a deny mode that would conflict with * current access? @@ -4017,11 +4022,21 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c __be32 status; unsigned char old_deny_bmap = stp->st_deny_bmap; - if (!test_access(open->op_share_access, stp)) - return nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open); +again: + spin_lock(&fp->fi_lock); + if (!test_access(open->op_share_access, stp)) { + spin_unlock(&fp->fi_lock); + status = nfs4_get_vfs_file(rqstp, fp, cur_fh, stp, open); + /* + * Somebody won the race for access while we did not hold + * the lock here + */ + if (status == nfserr_eagain) + goto again; + return status; + } /* test and set deny mode */ - spin_lock(&fp->fi_lock); status = nfs4_file_check_deny(fp, open->op_share_deny); if (status == nfs_ok) { set_deny(open->op_share_deny, stp); @@ -4361,6 +4376,13 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf status = nfs4_get_vfs_file(rqstp, fp, current_fh, stp, open); if (status) { up_read(&stp->st_rwsem); + /* + * EAGAIN is returned when there's a racing access, + * this should never happen as we are the only user + * of this new state, and since it's not yet hashed, + * nobody can find it + */ + WARN_ON(status == nfserr_eagain); release_open_stateid(stp); goto out; }