From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907309 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C098AC43334 for ; Wed, 6 Jul 2022 04:20:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231386AbiGFEUA (ORCPT ); Wed, 6 Jul 2022 00:20:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42622 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230359AbiGFET7 (ORCPT ); Wed, 6 Jul 2022 00:19:59 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F2B1A272C for ; Tue, 5 Jul 2022 21:19:57 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id A712722106; Wed, 6 Jul 2022 04:19:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081196; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rzy6MFguVAbNkbOictP0ahE6dTKboejlODTLHzbykFg=; b=In2IdmxzSoWFmeYEsjjcJ033eyuV5/yj/RMG8eAxK5t0jouPtkj6cXOCT+8r9663VbWvK5 lF8bliQnVLM9s04wPryGuzkA7kg8dJFIFwoN7ozfwezJIPFjdyB5IlLa3ImGENcwF0tz7F DaFYxKSVtyhXrDBukqz0O+SypMuxS2g= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081196; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rzy6MFguVAbNkbOictP0ahE6dTKboejlODTLHzbykFg=; b=nsL5qit9MJCukqMN8JafajH5m76P/OPuX+Xfnnym//Pn4sfFzZlCK3sHeEUd/k6+tJOxgI 8oew3nRyy0/bbHCQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 82A4713A37; Wed, 6 Jul 2022 04:19:55 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id ASsLD2sNxWKdQQAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:19:55 +0000 Subject: [PATCH 1/8] NFSD: drop rqstp arg to do_set_nfs4_acl() From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109255.1940.15894552631928142042.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org do_set_nfs4_acl() only needs rqstp to pass to nfsd4_set_nfs4_acl() The latter only needs the rqstp to pass to fh_verify(). In every case that do_set_nfs4_acl() is called, fh_verify() is not needed. It is only needed for filehandles received from the client, the filehandles passed to do_set_nfs4_acl() have just been constructed by the server, and so must be valid. So we can change nfsd4_set_nfs4_acl() to only call fh_verify() is rqstp is not NULL, and always pass NULL from do_set_nfs4_acl(). Signed-off-by: NeilBrown Reviewed-by: Jeff Layton --- fs/nfsd/nfs4acl.c | 12 +++++++----- fs/nfsd/nfs4proc.c | 9 ++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c index eaa3a0cf38f1..5c9b7e01e8ca 100644 --- a/fs/nfsd/nfs4acl.c +++ b/fs/nfsd/nfs4acl.c @@ -753,7 +753,7 @@ static int nfs4_acl_nfsv4_to_posix(struct nfs4_acl *acl, __be32 nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp, - struct nfs4_acl *acl) + struct nfs4_acl *acl) { __be32 error; int host_error; @@ -762,10 +762,12 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp, struct posix_acl *pacl = NULL, *dpacl = NULL; unsigned int flags = 0; - /* Get inode */ - error = fh_verify(rqstp, fhp, 0, NFSD_MAY_SATTR); - if (error) - return error; + if (rqstp) { + /* Get inode */ + error = fh_verify(rqstp, fhp, 0, NFSD_MAY_SATTR); + if (error) + return error; + } dentry = fhp->fh_dentry; inode = d_inode(dentry); diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 5af9f8d1feb6..60591ceb4985 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -163,12 +163,11 @@ is_create_with_attrs(struct nfsd4_open *open) * in the returned attr bitmap. */ static void -do_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp, - struct nfs4_acl *acl, u32 *bmval) +do_set_nfs4_acl(struct svc_fh *fhp, struct nfs4_acl *acl, u32 *bmval) { __be32 status; - status = nfsd4_set_nfs4_acl(rqstp, fhp, acl); + status = nfsd4_set_nfs4_acl(NULL, fhp, acl); if (status) /* * We should probably fail the whole open at this point, @@ -474,7 +473,7 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru goto out; if (is_create_with_attrs(open) && open->op_acl != NULL) - do_set_nfs4_acl(rqstp, *resfh, open->op_acl, open->op_bmval); + do_set_nfs4_acl(*resfh, open->op_acl, open->op_bmval); nfsd4_set_open_owner_reply_cache(cstate, open, *resfh); accmode = NFSD_MAY_NOP; @@ -861,7 +860,7 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, nfsd4_security_inode_setsecctx(&resfh, &create->cr_label, create->cr_bmval); if (create->cr_acl != NULL) - do_set_nfs4_acl(rqstp, &resfh, create->cr_acl, + do_set_nfs4_acl(&resfh, create->cr_acl, create->cr_bmval); fh_unlock(&cstate->current_fh); From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907310 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CC735C433EF for ; Wed, 6 Jul 2022 04:20:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231344AbiGFEUL (ORCPT ); Wed, 6 Jul 2022 00:20:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42654 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231592AbiGFEUF (ORCPT ); Wed, 6 Jul 2022 00:20:05 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 69CB7272C for ; Tue, 5 Jul 2022 21:20:03 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 27B192263C; Wed, 6 Jul 2022 04:20:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081202; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xQ7sfgP2oP0hobKYUIj/20wWeYkDJVH142Ry6LarLU8=; b=imEaAY/OvRtahQ9fJuvBngdpBs99ILhCnYlXjrz8vEJw9pInkrfxAHnghlvD86bkGZHXln FhuRodPg67GFNgu5GQ8hN82oLYWZ0wV5TFwFsylJHdFN5oDzTH4RcN3cdWjaJ7EnMgBAb2 3Sp3qexjF0k6cGxYi4+dFqrHhNvme1U= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081202; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xQ7sfgP2oP0hobKYUIj/20wWeYkDJVH142Ry6LarLU8=; b=ju3ujQtpbw9cGv+1AJrUYL2Ay2FXOEOJqfPL5J4c7HYPUQpPE0kCiS+mFPlqmyFCaSmqOJ Dyq5AKGTZVdRV9Aw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id D053113A37; Wed, 6 Jul 2022 04:20:00 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id JjSsI3ANxWKmQQAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:20:00 +0000 Subject: [PATCH 2/8] NFSD: change nfsd_create() to unlock directory before returning. From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109257.1940.11051756818329976731.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org nfsd_create() usually exits with the directory still locked. This relies on other code to unlock the directory. Planned future patches will change how directory locking works so the unlock step may be less trivial. It is cleaner to have lock and unlock in the same function. nfsd4_create() performs some extra changes after the creation and before the unlock - setting security label and an ACL. To allow for these to still be done while locked, we create a function nfsd4_post_create() and pass it to nfsd_create() when needed. nfsd_symlink() DOES usually unlock the directory, but nfsd4_create() may add a label or ACL - with the directory unlocked. I don't think symlinks have ACLs and don't know if they can have labels, so I don't know if this is of any practical consequence. For consistency nfsd_symlink() is changed to accept the same callback and call it if given. nfsd_symlink() didn't unlock the directory if lookup_one_len() gave an error. This is untidy and potentially confusing, and has now been fixed. It isn't a practical problem as an eventual fh_put() will unlock if needed. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton --- fs/nfsd/nfs3proc.c | 11 ++++++----- fs/nfsd/nfs4proc.c | 38 ++++++++++++++++++++++++-------------- fs/nfsd/nfsproc.c | 5 +++-- fs/nfsd/vfs.c | 40 +++++++++++++++++++++++++++------------- fs/nfsd/vfs.h | 11 ++++++++--- 5 files changed, 68 insertions(+), 37 deletions(-) diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index 981a3a7a6e16..38255365ef71 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -378,8 +378,8 @@ nfsd3_proc_mkdir(struct svc_rqst *rqstp) fh_copy(&resp->dirfh, &argp->fh); fh_init(&resp->fh, NFS3_FHSIZE); resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len, - &argp->attrs, S_IFDIR, 0, &resp->fh); - fh_unlock(&resp->dirfh); + &argp->attrs, S_IFDIR, 0, &resp->fh, + NULL, NULL); return rpc_success; } @@ -414,7 +414,8 @@ nfsd3_proc_symlink(struct svc_rqst *rqstp) fh_copy(&resp->dirfh, &argp->ffh); fh_init(&resp->fh, NFS3_FHSIZE); resp->status = nfsd_symlink(rqstp, &resp->dirfh, argp->fname, - argp->flen, argp->tname, &resp->fh); + argp->flen, argp->tname, &resp->fh, + NULL, NULL); kfree(argp->tname); out: return rpc_success; @@ -453,8 +454,8 @@ nfsd3_proc_mknod(struct svc_rqst *rqstp) type = nfs3_ftypes[argp->ftype]; resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len, - &argp->attrs, type, rdev, &resp->fh); - fh_unlock(&resp->dirfh); + &argp->attrs, type, rdev, &resp->fh, + NULL, NULL); out: return rpc_success; } diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 60591ceb4985..3279daab909d 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -780,6 +780,18 @@ nfsd4_commit(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, (__be32 *)commit->co_verf.data); } +static void nfsd4_post_create(struct svc_fh *fh, void *vcreate) +{ + struct nfsd4_create *create = vcreate; + + if (create->cr_label.len) + nfsd4_security_inode_setsecctx(fh, &create->cr_label, + create->cr_bmval); + + if (create->cr_acl != NULL) + do_set_nfs4_acl(fh, create->cr_acl, create->cr_bmval); +} + static __be32 nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, union nfsd4_op_u *u) @@ -805,7 +817,8 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, case NF4LNK: status = nfsd_symlink(rqstp, &cstate->current_fh, create->cr_name, create->cr_namelen, - create->cr_data, &resfh); + create->cr_data, &resfh, + nfsd4_post_create, create); break; case NF4BLK: @@ -816,7 +829,8 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, goto out_umask; status = nfsd_create(rqstp, &cstate->current_fh, create->cr_name, create->cr_namelen, - &create->cr_iattr, S_IFBLK, rdev, &resfh); + &create->cr_iattr, S_IFBLK, rdev, &resfh, + nfsd4_post_create, create); break; case NF4CHR: @@ -827,26 +841,30 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, goto out_umask; status = nfsd_create(rqstp, &cstate->current_fh, create->cr_name, create->cr_namelen, - &create->cr_iattr, S_IFCHR, rdev, &resfh); + &create->cr_iattr, S_IFCHR, rdev, &resfh, + nfsd4_post_create, create); break; case NF4SOCK: status = nfsd_create(rqstp, &cstate->current_fh, create->cr_name, create->cr_namelen, - &create->cr_iattr, S_IFSOCK, 0, &resfh); + &create->cr_iattr, S_IFSOCK, 0, &resfh, + nfsd4_post_create, create); break; case NF4FIFO: status = nfsd_create(rqstp, &cstate->current_fh, create->cr_name, create->cr_namelen, - &create->cr_iattr, S_IFIFO, 0, &resfh); + &create->cr_iattr, S_IFIFO, 0, &resfh, + nfsd4_post_create, create); break; case NF4DIR: create->cr_iattr.ia_valid &= ~ATTR_SIZE; status = nfsd_create(rqstp, &cstate->current_fh, create->cr_name, create->cr_namelen, - &create->cr_iattr, S_IFDIR, 0, &resfh); + &create->cr_iattr, S_IFDIR, 0, &resfh, + nfsd4_post_create, create); break; default: @@ -856,14 +874,6 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, if (status) goto out; - if (create->cr_label.len) - nfsd4_security_inode_setsecctx(&resfh, &create->cr_label, create->cr_bmval); - - if (create->cr_acl != NULL) - do_set_nfs4_acl(&resfh, create->cr_acl, - create->cr_bmval); - - fh_unlock(&cstate->current_fh); set_change_info(&create->cr_cinfo, &cstate->current_fh); fh_dup2(&cstate->current_fh, &resfh); out: diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c index fcdab8a8a41f..a25b8e321662 100644 --- a/fs/nfsd/nfsproc.c +++ b/fs/nfsd/nfsproc.c @@ -493,7 +493,7 @@ nfsd_proc_symlink(struct svc_rqst *rqstp) fh_init(&newfh, NFS_FHSIZE); resp->status = nfsd_symlink(rqstp, &argp->ffh, argp->fname, argp->flen, - argp->tname, &newfh); + argp->tname, &newfh, NULL, NULL); kfree(argp->tname); fh_put(&argp->ffh); @@ -522,7 +522,8 @@ nfsd_proc_mkdir(struct svc_rqst *rqstp) argp->attrs.ia_valid &= ~ATTR_SIZE; fh_init(&resp->fh, NFS_FHSIZE); resp->status = nfsd_create(rqstp, &argp->fh, argp->name, argp->len, - &argp->attrs, S_IFDIR, 0, &resp->fh); + &argp->attrs, S_IFDIR, 0, &resp->fh, + NULL, NULL); fh_put(&argp->fh); if (resp->status != nfs_ok) goto out; diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index d79db56475d4..1e7ca39e8a49 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1366,8 +1366,10 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp, */ __be32 nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, - char *fname, int flen, struct iattr *iap, - int type, dev_t rdev, struct svc_fh *resfhp) + char *fname, int flen, struct iattr *iap, + int type, dev_t rdev, struct svc_fh *resfhp, + void (*post_create)(struct svc_fh *fh, void *data), + void *data) { struct dentry *dentry, *dchild = NULL; __be32 err; @@ -1389,8 +1391,10 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, fh_lock_nested(fhp, I_MUTEX_PARENT); dchild = lookup_one_len(fname, dentry, flen); host_err = PTR_ERR(dchild); - if (IS_ERR(dchild)) - return nfserrno(host_err); + if (IS_ERR(dchild)) { + err = nfserrno(host_err); + goto out_unlock; + } err = fh_compose(resfhp, fhp->fh_export, dchild, fhp); /* * We unconditionally drop our ref to dchild as fh_compose will have @@ -1398,9 +1402,14 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, */ dput(dchild); if (err) - return err; - return nfsd_create_locked(rqstp, fhp, fname, flen, iap, type, - rdev, resfhp); + goto out_unlock; + err = nfsd_create_locked(rqstp, fhp, fname, flen, iap, type, + rdev, resfhp); + if (!err && post_create) + post_create(resfhp, data); +out_unlock: + fh_unlock(fhp); + return err; } /* @@ -1447,9 +1456,11 @@ nfsd_readlink(struct svc_rqst *rqstp, struct svc_fh *fhp, char *buf, int *lenp) */ __be32 nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp, - char *fname, int flen, - char *path, - struct svc_fh *resfhp) + char *fname, int flen, + char *path, + struct svc_fh *resfhp, + void (*post_create)(struct svc_fh *fh, void *data), + void *data) { struct dentry *dentry, *dnew; __be32 err, cerr; @@ -1474,12 +1485,12 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp, dentry = fhp->fh_dentry; dnew = lookup_one_len(fname, dentry, flen); host_err = PTR_ERR(dnew); - if (IS_ERR(dnew)) + if (IS_ERR(dnew)) { + fh_unlock(fhp); goto out_nfserr; - + } host_err = vfs_symlink(&init_user_ns, d_inode(dentry), dnew, path); err = nfserrno(host_err); - fh_unlock(fhp); if (!err) err = nfserrno(commit_metadata(fhp)); @@ -1488,6 +1499,9 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp, cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp); dput(dnew); if (err==0) err = cerr; + if (!err && post_create) + post_create(resfhp, data); + fh_unlock(fhp); out: return err; diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h index 26347d76f44a..9f4fd3060200 100644 --- a/fs/nfsd/vfs.h +++ b/fs/nfsd/vfs.h @@ -66,8 +66,10 @@ __be32 nfsd_create_locked(struct svc_rqst *, struct svc_fh *, char *name, int len, struct iattr *attrs, int type, dev_t rdev, struct svc_fh *res); __be32 nfsd_create(struct svc_rqst *, struct svc_fh *, - char *name, int len, struct iattr *attrs, - int type, dev_t rdev, struct svc_fh *res); + char *name, int len, struct iattr *attrs, + int type, dev_t rdev, struct svc_fh *res, + void (*post_create)(struct svc_fh *fh, void *data), + void *data); __be32 nfsd_access(struct svc_rqst *, struct svc_fh *, u32 *, u32 *); __be32 nfsd_create_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct svc_fh *resfhp, struct iattr *iap); @@ -111,7 +113,10 @@ __be32 nfsd_readlink(struct svc_rqst *, struct svc_fh *, char *, int *); __be32 nfsd_symlink(struct svc_rqst *, struct svc_fh *, char *name, int len, char *path, - struct svc_fh *res); + struct svc_fh *res, + void (*post_create)(struct svc_fh *fh, + void *data), + void *data); __be32 nfsd_link(struct svc_rqst *, struct svc_fh *, char *, int, struct svc_fh *); ssize_t nfsd_copy_file_range(struct file *, u64, From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907311 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E6A73C43334 for ; Wed, 6 Jul 2022 04:20:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231592AbiGFEUP (ORCPT ); Wed, 6 Jul 2022 00:20:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42694 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230359AbiGFEUN (ORCPT ); Wed, 6 Jul 2022 00:20:13 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B825B272C for ; Tue, 5 Jul 2022 21:20:12 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 7B07C1F8B6; Wed, 6 Jul 2022 04:20:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081211; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=V0ONAXNcsuNCM3u/6sv99D1JZhgOVP6Sd2g1CyRBucw=; b=Tb2G10wlMKonC3BsUuq9kRysePDfoocgrAok3mxud6aXt7hxcQTu6sSAUWEG+rT5t/8JjR oYrx7fmNKjuq+cv+XWiAa1YYXQLLUQ+j0v5JCY287HhtEUl3YZ7RKAFJHYCJXZfRJ/KwEa dpefM2sAXOzdqLpUceCoqy1HSuoU0I0= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081211; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=V0ONAXNcsuNCM3u/6sv99D1JZhgOVP6Sd2g1CyRBucw=; b=LKkwdYqCC9939Q+zji4oDzr1r5J4sDrU03FYYIpbpW4FnNv2k4QWsGK4VAssNeQLDhipw/ YCY7uOv85hmC3wAw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id A923213A37; Wed, 6 Jul 2022 04:20:07 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id qvVGGXcNxWK2QQAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:20:07 +0000 Subject: [PATCH 3/8] NFSD: always drop directory lock in nfsd_unlink() From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109257.1940.5635801592688577227.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org Some error paths in nfsd_unlink() allow it to exit without unlocking the directory. This is not a problem in practice as the directory will be locked with an fh_put(), but it is untidy and potentially confusing. This allows us to remove all the fh_unlock() calls that are immediately after nfsd_unlink() calls. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton --- fs/nfsd/nfs3proc.c | 2 -- fs/nfsd/nfs4proc.c | 4 +--- fs/nfsd/vfs.c | 7 +++++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index 38255365ef71..ad7941001106 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -478,7 +478,6 @@ nfsd3_proc_remove(struct svc_rqst *rqstp) fh_copy(&resp->fh, &argp->fh); resp->status = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR, argp->name, argp->len); - fh_unlock(&resp->fh); return rpc_success; } @@ -499,7 +498,6 @@ nfsd3_proc_rmdir(struct svc_rqst *rqstp) fh_copy(&resp->fh, &argp->fh); resp->status = nfsd_unlink(rqstp, &resp->fh, S_IFDIR, argp->name, argp->len); - fh_unlock(&resp->fh); return rpc_success; } diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 3279daab909d..4737019738ab 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1052,10 +1052,8 @@ nfsd4_remove(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, return nfserr_grace; status = nfsd_unlink(rqstp, &cstate->current_fh, 0, remove->rm_name, remove->rm_namelen); - if (!status) { - fh_unlock(&cstate->current_fh); + if (!status) set_change_info(&remove->rm_cinfo, &cstate->current_fh); - } return status; } diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 1e7ca39e8a49..3f4579f5775c 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1762,12 +1762,12 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, rdentry = lookup_one_len(fname, dentry, flen); host_err = PTR_ERR(rdentry); if (IS_ERR(rdentry)) - goto out_drop_write; + goto out_unlock; if (d_really_is_negative(rdentry)) { dput(rdentry); host_err = -ENOENT; - goto out_drop_write; + goto out_unlock; } rinode = d_inode(rdentry); ihold(rinode); @@ -1805,6 +1805,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, } out: return err; +out_unlock: + fh_unlock(fhp); + goto out_drop_write; } /* From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907312 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5FF78C43334 for ; Wed, 6 Jul 2022 04:20:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231349AbiGFEUs (ORCPT ); Wed, 6 Jul 2022 00:20:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43094 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229921AbiGFEUr (ORCPT ); Wed, 6 Jul 2022 00:20:47 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D25921838E for ; Tue, 5 Jul 2022 21:20:46 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 84ACC22849; Wed, 6 Jul 2022 04:20:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081245; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=w7LQxZ6st6gsPWv1TDn+iaNfRHm08fMBBxCDWaA9wrE=; b=F6A2QvJ7XvYoc8co6F7BhNSO2xXdBaFnpi6ekoiLRg7M1WMvVHNF19gYvCj2yzfrJV7yyP 7bnowXPmsqkZcyj6j2ayb4rQclQY6MfDmM3kkikqY3XMZR40cM9l6nL/XG1G51h+HOHnVC 0Qrglm0FaJKyeZLBD4/5j37QBRxVMWs= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081245; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=w7LQxZ6st6gsPWv1TDn+iaNfRHm08fMBBxCDWaA9wrE=; b=ZHtR5xZIlGqsE+jMJeUBnupClZc1vB82zpemIYzpAMVX2hhqG/RoOkCuM0UqYjgyXBsbRm YqXJAaa/WVjiCXCw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 67E4513A37; Wed, 6 Jul 2022 04:20:44 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 0IgWCZwNxWLvQQAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:20:44 +0000 Subject: [PATCH 4/8] NFSD: only call fh_unlock() once in nfsd_link() From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109258.1940.6581517569232462503.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org On non-error paths, nfsd_link() calls fh_unlock() twice. This is safe because fh_unlock() records that the unlock has been done and doesn't repeat it. However it makes the code a little confusing and interferes with changes that are planned for directory locking. So rearrange the code to ensure fh_unlock() is called exactly once if fh_lock() was called. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton --- fs/nfsd/vfs.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 3f4579f5775c..4916c29af0fa 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1551,8 +1551,10 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, dnew = lookup_one_len(name, ddir, len); host_err = PTR_ERR(dnew); - if (IS_ERR(dnew)) - goto out_nfserr; + if (IS_ERR(dnew)) { + err = nfserrno(host_err); + goto out_unlock; + } dold = tfhp->fh_dentry; @@ -1571,17 +1573,17 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, else err = nfserrno(host_err); } -out_dput: dput(dnew); -out_unlock: - fh_unlock(ffhp); +out_drop_write: fh_drop_write(tfhp); out: return err; -out_nfserr: - err = nfserrno(host_err); - goto out_unlock; +out_dput: + dput(dnew); +out_unlock: + fh_unlock(ffhp); + goto out_drop_write; } static void From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907313 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F3296C433EF for ; Wed, 6 Jul 2022 04:20:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229921AbiGFEUx (ORCPT ); Wed, 6 Jul 2022 00:20:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43140 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231661AbiGFEUx (ORCPT ); Wed, 6 Jul 2022 00:20:53 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7ED48272C for ; Tue, 5 Jul 2022 21:20:51 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 3C2681F8B6; Wed, 6 Jul 2022 04:20:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081250; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C1a8yo+4tfdZ9zOZuai3dvqhEuaLma88FnlthRvqO+U=; b=vHYFeqPTRrzhalOl7DhOkDXUwAsIzIrrf3HGrMf9wrTqpN/hnCWHdLIcudGKD4+BPHXGLj c0659OkVbhP6ZisviRbG2UxmwsotP0JMYVIgNQVQcBQTawtIWE6dyYpwmTwdRnYOLLhf3j 8cWp3e6YAKbz30Q0sl+SwtxbGaAtVfY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081250; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C1a8yo+4tfdZ9zOZuai3dvqhEuaLma88FnlthRvqO+U=; b=F/lqcC2Ih4ZWI/W2yk6dTx+oGlJsVp3Wi5M8oKVr0KLYPr2DWqSIp+auzSvmNLPypXNx32 CGlBS+VJ3pbYqsCg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id D37E913A37; Wed, 6 Jul 2022 04:20:48 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 4OyJI6ANxWL4QQAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:20:48 +0000 Subject: [PATCH 5/8] NFSD: reduce locking in nfsd_lookup() From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109258.1940.1095066282860556838.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org nfsd_lookup() takes an exclusive lock on the parent inode, but many callers don't want the lock and may not need to lock at all if the result is in the dcache. Change nfsd_lookup() to be passed a bool flag. If false, don't take the lock. If true, do take an exclusive lock, and return with it held if successful. If nfsd_lookup() returns an error, the lock WILL NOT be held. Only nfsd4_open() requests the lock to be held, and does so to block rename until it decides whether to return a delegation. NOTE: when nfsd4_open() creates a file, the directory does *NOT* remain locked and never has. So it is possible (though unlikely) for the newly created file to be renamed before a delegation is handed out, and that would be bad. This patch does *NOT* fix that, but *DOES* take the directory lock immediately after creating the file, which reduces the size of the window and ensure that the lock is held consistently. More work is needed to guarantee no rename happens before the delegation. NOTE-2: NFSv4 requires directory changeinfo for OPEN even when a create wasn't requested and no change happened. Now that nfsd_lookup() doesn't use fh_lock(), we need explicit fh_fill_pre/post_attrs() in the non-create branch of do_open_lookup(). Signed-off-by: NeilBrown Reviewed-by: Jeff Layton --- fs/nfsd/nfs3proc.c | 2 +- fs/nfsd/nfs4proc.c | 51 ++++++++++++++++++++++++++++------------ fs/nfsd/nfsproc.c | 2 +- fs/nfsd/vfs.c | 66 +++++++++++++++++++++++++++++++++++----------------- fs/nfsd/vfs.h | 8 ++++-- 5 files changed, 88 insertions(+), 41 deletions(-) diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index ad7941001106..3a67d0afb885 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -96,7 +96,7 @@ nfsd3_proc_lookup(struct svc_rqst *rqstp) resp->status = nfsd_lookup(rqstp, &resp->dirfh, argp->name, argp->len, - &resp->fh); + &resp->fh, false); return rpc_success; } diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 4737019738ab..6ec22c69cbec 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -414,7 +414,8 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, } static __be32 -do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_open *open, struct svc_fh **resfh) +do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, + struct nfsd4_open *open, struct svc_fh **resfh) { struct svc_fh *current_fh = &cstate->current_fh; int accmode; @@ -441,11 +442,18 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru * yes | no | GUARDED4 | GUARDED4 * yes | yes | GUARDED4 | GUARDED4 */ - current->fs->umask = open->op_umask; status = nfsd4_create_file(rqstp, current_fh, *resfh, open); current->fs->umask = 0; + if (!status) + /* We really want to hold the lock from before the + * create to ensure no rename happens, but that + * needs more work... + */ + inode_lock_nested(current_fh->fh_dentry->d_inode, + I_MUTEX_PARENT); + if (!status && open->op_label.len) nfsd4_security_inode_setsecctx(*resfh, &open->op_label, open->op_bmval); @@ -457,17 +465,25 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru if (nfsd4_create_is_exclusive(open->op_createmode) && status == 0) open->op_bmval[1] |= (FATTR4_WORD1_TIME_ACCESS | FATTR4_WORD1_TIME_MODIFY); - } else - /* - * Note this may exit with the parent still locked. - * We will hold the lock until nfsd4_open's final - * lookup, to prevent renames or unlinks until we've had - * a chance to an acquire a delegation if appropriate. + } else { + /* We want to keep the directory locked until we've had a chance + * to acquire a delegation if appropriate, so request that + * nfsd_lookup() hold on to the lock. */ status = nfsd_lookup(rqstp, current_fh, - open->op_fname, open->op_fnamelen, *resfh); + open->op_fname, open->op_fnamelen, *resfh, + true); + if (!status) { + /* NFSv4 protocol requires change attributes even though + * no change happened. + */ + fh_fill_pre_attrs(current_fh); + fh_fill_post_attrs(current_fh); + } + } if (status) - goto out; + return status; + status = nfsd_check_obj_isreg(*resfh); if (status) goto out; @@ -483,6 +499,8 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru status = do_open_permission(rqstp, *resfh, open, accmode); set_change_info(&open->op_cinfo, current_fh); out: + if (status) + inode_unlock(current_fh->fh_dentry->d_inode); return status; } @@ -540,6 +558,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct net *net = SVC_NET(rqstp); struct nfsd_net *nn = net_generic(net, nfsd_net_id); bool reclaim = false; + bool locked = false; dprintk("NFSD: nfsd4_open filename %.*s op_openowner %p\n", (int)open->op_fnamelen, open->op_fname, @@ -604,6 +623,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, status = do_open_lookup(rqstp, cstate, open, &resfh); if (status) goto out; + locked = true; break; case NFS4_OPEN_CLAIM_PREVIOUS: status = nfs4_check_open_reclaim(cstate->clp); @@ -639,6 +659,8 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, fput(open->op_filp); open->op_filp = NULL; } + if (locked) + inode_unlock(cstate->current_fh.fh_dentry->d_inode); if (resfh && resfh != &cstate->current_fh) { fh_dup2(&cstate->current_fh, resfh); fh_put(resfh); @@ -933,7 +955,7 @@ static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh) return nfserr_noent; } fh_put(&tmp_fh); - return nfsd_lookup(rqstp, fh, "..", 2, fh); + return nfsd_lookup(rqstp, fh, "..", 2, fh, false); } static __be32 @@ -949,7 +971,7 @@ nfsd4_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, { return nfsd_lookup(rqstp, &cstate->current_fh, u->lookup.lo_name, u->lookup.lo_len, - &cstate->current_fh); + &cstate->current_fh, false); } static __be32 @@ -1089,11 +1111,10 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, if (err) return err; err = nfsd_lookup_dentry(rqstp, &cstate->current_fh, - secinfo->si_name, secinfo->si_namelen, - &exp, &dentry); + secinfo->si_name, secinfo->si_namelen, + &exp, &dentry, false); if (err) return err; - fh_unlock(&cstate->current_fh); if (d_really_is_negative(dentry)) { exp_put(exp); err = nfserr_noent; diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c index a25b8e321662..ed24fae09517 100644 --- a/fs/nfsd/nfsproc.c +++ b/fs/nfsd/nfsproc.c @@ -133,7 +133,7 @@ nfsd_proc_lookup(struct svc_rqst *rqstp) fh_init(&resp->fh, NFS_FHSIZE); resp->status = nfsd_lookup(rqstp, &argp->fh, argp->name, argp->len, - &resp->fh); + &resp->fh, false); fh_put(&argp->fh); if (resp->status != nfs_ok) goto out; diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 4916c29af0fa..8e050c6d112a 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -172,7 +172,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp) __be32 nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name, unsigned int len, - struct svc_export **exp_ret, struct dentry **dentry_ret) + struct svc_export **exp_ret, struct dentry **dentry_ret, + bool locked) { struct svc_export *exp; struct dentry *dparent; @@ -199,27 +200,31 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp, goto out_nfserr; } } else { - /* - * In the nfsd4_open() case, this may be held across - * subsequent open and delegation acquisition which may - * need to take the child's i_mutex: - */ - fh_lock_nested(fhp, I_MUTEX_PARENT); - dentry = lookup_one_len(name, dparent, len); + if (locked) + dentry = lookup_one_len(name, dparent, len); + else + dentry = lookup_one_len_unlocked(name, dparent, len); host_err = PTR_ERR(dentry); if (IS_ERR(dentry)) goto out_nfserr; if (nfsd_mountpoint(dentry, exp)) { /* - * We don't need the i_mutex after all. It's - * still possible we could open this (regular - * files can be mountpoints too), but the - * i_mutex is just there to prevent renames of - * something that we might be about to delegate, - * and a mountpoint won't be renamed: + * nfsd_cross_mnt() may wait for an upcall + * to userspace, and holding i_sem across that + * invites the possibility of a deadlock. + * We don't really need the lock on the parent + * of a mount point was we only need it to guard + * against a rename before we get a lease for a + * delegation. + * So just drop the i_sem and reclaim it. */ - fh_unlock(fhp); - if ((host_err = nfsd_cross_mnt(rqstp, &dentry, &exp))) { + if (locked) + inode_unlock(dparent->d_inode); + host_err = nfsd_cross_mnt(rqstp, &dentry, &exp); + if (locked) + inode_lock_nested(dparent->d_inode, + I_MUTEX_PARENT); + if (host_err) { dput(dentry); goto out_nfserr; } @@ -234,7 +239,17 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp, return nfserrno(host_err); } -/* +/** + * nfsd_lookup - look up a single path component for nfsd + * + * @rqstp: the request context + * @ftp: the file handle of the directory + * @name: the component name, or %NULL to look up parent + * @len: length of name to examine + * @resfh: pointer to pre-initialised filehandle to hold result. + * @lock: if true, lock directory during lookup and keep it locked + * if there is no error. + * * Look up one component of a pathname. * N.B. After this call _both_ fhp and resfh need an fh_put * @@ -244,11 +259,15 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp, * returned. Otherwise the covered directory is returned. * NOTE: this mountpoint crossing is not supported properly by all * clients and is explicitly disallowed for NFSv3 - * NeilBrown + * + * Only nfsd4_open() calls this with @lock set. It does so to block + * renames/unlinks before it possibly gets a lease to provide a + * delegation. */ __be32 nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name, - unsigned int len, struct svc_fh *resfh) + unsigned int len, struct svc_fh *resfh, + bool lock) { struct svc_export *exp; struct dentry *dentry; @@ -257,9 +276,11 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name, err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC); if (err) return err; - err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry); + if (lock) + inode_lock_nested(fhp->fh_dentry->d_inode, I_MUTEX_PARENT); + err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry, lock); if (err) - return err; + goto out_err; err = check_nfsd_access(exp, rqstp); if (err) goto out; @@ -273,6 +294,9 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name, out: dput(dentry); exp_put(exp); +out_err: + if (err && lock) + inode_unlock(fhp->fh_dentry->d_inode); return err; } diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h index 9f4fd3060200..290788f007d4 100644 --- a/fs/nfsd/vfs.h +++ b/fs/nfsd/vfs.h @@ -45,10 +45,12 @@ typedef int (*nfsd_filldir_t)(void *, const char *, int, loff_t, u64, unsigned); int nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp, struct svc_export **expp); __be32 nfsd_lookup(struct svc_rqst *, struct svc_fh *, - const char *, unsigned int, struct svc_fh *); + const char *, unsigned int, struct svc_fh *, + bool); __be32 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *, - const char *, unsigned int, - struct svc_export **, struct dentry **); + const char *, unsigned int, + struct svc_export **, struct dentry **, + bool); __be32 nfsd_setattr(struct svc_rqst *, struct svc_fh *, struct iattr *, int, time64_t); int nfsd_mountpoint(struct dentry *, struct svc_export *); From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907314 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E65B0C43334 for ; Wed, 6 Jul 2022 04:21:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230274AbiGFEU7 (ORCPT ); Wed, 6 Jul 2022 00:20:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43194 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231645AbiGFEU6 (ORCPT ); Wed, 6 Jul 2022 00:20:58 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5B87E1838E for ; Tue, 5 Jul 2022 21:20:57 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 1A7B6220A7; Wed, 6 Jul 2022 04:20:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081256; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CzVpTHNcBPiNz9K9S7ljnD9RLTSaAjbmyLozYzpHiPs=; b=zj4qKLCGdXjD1qFlixdNKnh0GmyapTbHJNsppghdOf6KG1v9qd982YM9lfdCROgnrU03kH DqaTXQ946zegofj00/CMpYFcIYSXG8ukVFYNBv7HwreachRR1b1Y7LbxbUfJFd6GDN4k9o zecWoD6vYoNe3EjPgVDFo5Y29bQgjXQ= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081256; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CzVpTHNcBPiNz9K9S7ljnD9RLTSaAjbmyLozYzpHiPs=; b=nlP47roMvf0Qntj34btVS5Xj5ZYFkTHBJRkC4QtomwY4QjOZ3bMoqZRkhqX5iB7yW3gvM3 i9zevbXzGnEje6BQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 99C0E13A37; Wed, 6 Jul 2022 04:20:54 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id aJ/TFKYNxWL8QQAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:20:54 +0000 Subject: [PATCH 6/8] NFSD: use explicit lock/unlock for directory ops From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109259.1940.685583862810495747.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org When creating or unlinking a name in a directory use explicit inode_lock_nested() instead of fh_lock(), and explicit calls to fh_fill_pre_attrs() and fh_fill_post_attrs(). This is already done for renames. Also move the 'fill' calls closer to the operation that might change the attributes. This way they are avoided on some error paths. Having the locking explicit will simplify proposed future changes to locking for directories. It also makes it easily visible exactly where pre/post attributes are used - not all callers of fh_lock() actually need the pre/post attributes. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton Signed-off-by: Jeff Layton --- fs/nfsd/nfs3proc.c | 6 ++++-- fs/nfsd/nfs4proc.c | 6 ++++-- fs/nfsd/nfsproc.c | 7 ++++--- fs/nfsd/vfs.c | 30 +++++++++++++++++++----------- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index 3a67d0afb885..9629517344ff 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -254,7 +254,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, if (host_err) return nfserrno(host_err); - fh_lock_nested(fhp, I_MUTEX_PARENT); + inode_lock_nested(inode, I_MUTEX_PARENT); child = lookup_one_len(argp->name, parent, argp->len); if (IS_ERR(child)) { @@ -312,11 +312,13 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, if (!IS_POSIXACL(inode)) iap->ia_mode &= ~current_umask(); + fh_fill_pre_attrs(fhp); host_err = vfs_create(&init_user_ns, inode, child, iap->ia_mode, true); if (host_err < 0) { status = nfserrno(host_err); goto out; } + fh_fill_post_attrs(fhp); /* A newly created file already has a file size of zero. */ if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0)) @@ -334,7 +336,7 @@ nfsd3_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, status = nfsd_create_setattr(rqstp, fhp, resfhp, iap); out: - fh_unlock(fhp); + inode_unlock(inode); if (child && !IS_ERR(child)) dput(child); fh_drop_write(fhp); diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 6ec22c69cbec..242f059e6788 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -306,7 +306,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, if (host_err) return nfserrno(host_err); - fh_lock_nested(fhp, I_MUTEX_PARENT); + inode_lock_nested(inode, I_MUTEX_PARENT); child = lookup_one_len(open->op_fname, parent, open->op_fnamelen); if (IS_ERR(child)) { @@ -385,10 +385,12 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, if (!IS_POSIXACL(inode)) iap->ia_mode &= ~current_umask(); + fh_fill_pre_attrs(fhp); status = nfsd4_vfs_create(fhp, child, open); if (status != nfs_ok) goto out; open->op_created = true; + fh_fill_post_attrs(fhp); /* A newly created file already has a file size of zero. */ if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0)) @@ -406,7 +408,7 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, status = nfsd_create_setattr(rqstp, fhp, resfhp, iap); out: - fh_unlock(fhp); + inode_unlock(inode); if (child && !IS_ERR(child)) dput(child); fh_drop_write(fhp); diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c index ed24fae09517..427c404bc52b 100644 --- a/fs/nfsd/nfsproc.c +++ b/fs/nfsd/nfsproc.c @@ -285,7 +285,7 @@ nfsd_proc_create(struct svc_rqst *rqstp) goto done; } - fh_lock_nested(dirfhp, I_MUTEX_PARENT); + inode_lock_nested(dirfhp->fh_dentry->d_inode, I_MUTEX_PARENT); dchild = lookup_one_len(argp->name, dirfhp->fh_dentry, argp->len); if (IS_ERR(dchild)) { resp->status = nfserrno(PTR_ERR(dchild)); @@ -382,6 +382,7 @@ nfsd_proc_create(struct svc_rqst *rqstp) } resp->status = nfs_ok; + fh_fill_pre_attrs(dirfhp); if (!inode) { /* File doesn't exist. Create it and set attrs */ resp->status = nfsd_create_locked(rqstp, dirfhp, argp->name, @@ -399,10 +400,10 @@ nfsd_proc_create(struct svc_rqst *rqstp) resp->status = nfsd_setattr(rqstp, newfhp, attr, 0, (time64_t)0); } + fh_fill_post_attrs(dirfhp); out_unlock: - /* We don't really need to unlock, as fh_put does it. */ - fh_unlock(dirfhp); + inode_unlock(dirfhp->fh_dentry->d_inode); fh_drop_write(dirfhp); done: fh_put(dirfhp); diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 8e050c6d112a..2ca748aa83bb 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1412,7 +1412,7 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, if (host_err) return nfserrno(host_err); - fh_lock_nested(fhp, I_MUTEX_PARENT); + inode_lock_nested(dentry->d_inode, I_MUTEX_PARENT); dchild = lookup_one_len(fname, dentry, flen); host_err = PTR_ERR(dchild); if (IS_ERR(dchild)) { @@ -1427,12 +1427,14 @@ nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp, dput(dchild); if (err) goto out_unlock; + fh_fill_pre_attrs(fhp); err = nfsd_create_locked(rqstp, fhp, fname, flen, iap, type, rdev, resfhp); if (!err && post_create) post_create(resfhp, data); + fh_fill_post_attrs(fhp); out_unlock: - fh_unlock(fhp); + inode_unlock(dentry->d_inode); return err; } @@ -1505,14 +1507,15 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp, if (host_err) goto out_nfserr; - fh_lock(fhp); dentry = fhp->fh_dentry; + inode_lock_nested(dentry->d_inode, I_MUTEX_PARENT); dnew = lookup_one_len(fname, dentry, flen); host_err = PTR_ERR(dnew); if (IS_ERR(dnew)) { - fh_unlock(fhp); + inode_unlock(dentry->d_inode); goto out_nfserr; } + fh_fill_pre_attrs(fhp); host_err = vfs_symlink(&init_user_ns, d_inode(dentry), dnew, path); err = nfserrno(host_err); if (!err) @@ -1525,7 +1528,8 @@ nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp, if (err==0) err = cerr; if (!err && post_create) post_create(resfhp, data); - fh_unlock(fhp); + fh_fill_post_attrs(fhp); + inode_unlock(dentry->d_inode); out: return err; @@ -1569,9 +1573,9 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, goto out; } - fh_lock_nested(ffhp, I_MUTEX_PARENT); ddir = ffhp->fh_dentry; dirp = d_inode(ddir); + inode_lock_nested(dirp, I_MUTEX_PARENT); dnew = lookup_one_len(name, ddir, len); host_err = PTR_ERR(dnew); @@ -1585,8 +1589,10 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, err = nfserr_noent; if (d_really_is_negative(dold)) goto out_dput; + fh_fill_pre_attrs(ffhp); host_err = vfs_link(dold, &init_user_ns, dirp, dnew, NULL); - fh_unlock(ffhp); + fh_fill_post_attrs(ffhp); + inode_unlock(dirp); if (!host_err) { err = nfserrno(commit_metadata(ffhp)); if (!err) @@ -1606,7 +1612,7 @@ nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp, out_dput: dput(dnew); out_unlock: - fh_unlock(ffhp); + inode_unlock(dirp); goto out_drop_write; } @@ -1781,9 +1787,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, if (host_err) goto out_nfserr; - fh_lock_nested(fhp, I_MUTEX_PARENT); dentry = fhp->fh_dentry; dirp = d_inode(dentry); + inode_lock_nested(dirp, I_MUTEX_PARENT); rdentry = lookup_one_len(fname, dentry, flen); host_err = PTR_ERR(rdentry); @@ -1801,6 +1807,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, if (!type) type = d_inode(rdentry)->i_mode & S_IFMT; + fh_fill_pre_attrs(fhp); if (type != S_IFDIR) { if (rdentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK) nfsd_close_cached_files(rdentry); @@ -1808,8 +1815,9 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, } else { host_err = vfs_rmdir(&init_user_ns, dirp, rdentry); } + fh_fill_post_attrs(fhp); - fh_unlock(fhp); + inode_unlock(dirp); if (!host_err) host_err = commit_metadata(fhp); dput(rdentry); @@ -1832,7 +1840,7 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, out: return err; out_unlock: - fh_unlock(fhp); + inode_unlock(dirp); goto out_drop_write; } From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907315 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 57C48C433EF for ; Wed, 6 Jul 2022 04:21:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231130AbiGFEVb (ORCPT ); Wed, 6 Jul 2022 00:21:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43478 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231661AbiGFEVa (ORCPT ); Wed, 6 Jul 2022 00:21:30 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0EF521838E for ; Tue, 5 Jul 2022 21:21:29 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id BBC671F8D8; Wed, 6 Jul 2022 04:21:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081287; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hCsWxh1ujzkMG9C+r1JDX2CNmUtweUV3bl/Qbt4JkG8=; b=FNpSiP37LC1XG8jCmBAetjhPF9uJlA0GhEKwyMh3k8hiXFq4It4/gv5ctS8O98ybt1KaIE TDEI/5SXLc66A/7cH73XDXTlrrA/S2OnpI0Etto3ux5rLClOBd0dXobDG6Dxv/k6Wb2/Vp Svr0v3rGV4/X6lv+YOZRjXOSENyDf24= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081287; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hCsWxh1ujzkMG9C+r1JDX2CNmUtweUV3bl/Qbt4JkG8=; b=PW1TIFpJhyebaOWj1AD+r00LA5X87cFdy3xAx5+EA5vMFMkcUvyg/oLcLuK3oCOEpxdPVn HTS8Af25uc80c/CQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 9C6EA13A37; Wed, 6 Jul 2022 04:21:26 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id t6rdFcYNxWIjQgAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:21:26 +0000 Subject: [PATCH 7/8] NFSD: use (un)lock_inode instead of fh_(un)lock for file operations From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109260.1940.6599746560136720935.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org When locking a file to access ACLs and xattrs etc, use explicit locking with inode_lock() instead of fh_lock(). This means that the calls to fh_fill_pre/post_attr() are also explicit which improves readability and allows us to place them only where they are needed. Only the xattr calls need pre/post information. When locking a file we don't need I_MUTEX_PARENT as the file is not a parent of anything, so we can use inode_lock() directly rather than the inode_lock_nested() call that fh_lock() uses. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton --- fs/nfsd/nfs2acl.c | 6 +++--- fs/nfsd/nfs3acl.c | 4 ++-- fs/nfsd/nfs4acl.c | 7 +++---- fs/nfsd/nfs4state.c | 8 ++++---- fs/nfsd/vfs.c | 25 ++++++++++++------------- 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c index b5760801d377..9edd3c1a30fb 100644 --- a/fs/nfsd/nfs2acl.c +++ b/fs/nfsd/nfs2acl.c @@ -111,7 +111,7 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp) if (error) goto out_errno; - fh_lock(fh); + inode_lock(inode); error = set_posix_acl(&init_user_ns, inode, ACL_TYPE_ACCESS, argp->acl_access); @@ -122,7 +122,7 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp) if (error) goto out_drop_lock; - fh_unlock(fh); + inode_unlock(inode); fh_drop_write(fh); @@ -136,7 +136,7 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp) return rpc_success; out_drop_lock: - fh_unlock(fh); + inode_unlock(inode); fh_drop_write(fh); out_errno: resp->status = nfserrno(error); diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c index 35b2ebda14da..9446c6743664 100644 --- a/fs/nfsd/nfs3acl.c +++ b/fs/nfsd/nfs3acl.c @@ -101,7 +101,7 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst *rqstp) if (error) goto out_errno; - fh_lock(fh); + inode_lock(inode); error = set_posix_acl(&init_user_ns, inode, ACL_TYPE_ACCESS, argp->acl_access); @@ -111,7 +111,7 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst *rqstp) argp->acl_default); out_drop_lock: - fh_unlock(fh); + inode_unlock(inode); fh_drop_write(fh); out_errno: resp->status = nfserrno(error); diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c index 5c9b7e01e8ca..a33cacf62ea0 100644 --- a/fs/nfsd/nfs4acl.c +++ b/fs/nfsd/nfs4acl.c @@ -781,19 +781,18 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp, if (host_error < 0) goto out_nfserr; - fh_lock(fhp); + inode_lock(inode); host_error = set_posix_acl(&init_user_ns, inode, ACL_TYPE_ACCESS, pacl); if (host_error < 0) goto out_drop_lock; - if (S_ISDIR(inode->i_mode)) { + if (S_ISDIR(inode->i_mode)) host_error = set_posix_acl(&init_user_ns, inode, ACL_TYPE_DEFAULT, dpacl); - } out_drop_lock: - fh_unlock(fhp); + inode_unlock(inode); posix_acl_release(pacl); posix_acl_release(dpacl); diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 9d1a3e131c49..307317ba9aff 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -7322,21 +7322,21 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, static __be32 nfsd_test_lock(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file_lock *lock) { struct nfsd_file *nf; + struct inode *inode = fhp->fh_dentry->d_inode; __be32 err; err = nfsd_file_acquire(rqstp, fhp, NFSD_MAY_READ, &nf); if (err) return err; - fh_lock(fhp); /* to block new leases till after test_lock: */ - err = nfserrno(nfsd_open_break_lease(fhp->fh_dentry->d_inode, - NFSD_MAY_READ)); + inode_lock(inode); /* to block new leases till after test_lock: */ + err = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ)); if (err) goto out; lock->fl_file = nf->nf_file; err = nfserrno(vfs_test_lock(nf->nf_file, lock)); lock->fl_file = NULL; out: - fh_unlock(fhp); + inode_unlock(inode); nfsd_file_put(nf); return err; } diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 2ca748aa83bb..2526615285ca 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -444,7 +444,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap, return err; } - fh_lock(fhp); + inode_lock(inode); if (size_change) { /* * RFC5661, Section 18.30.4: @@ -480,7 +480,7 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap, host_err = notify_change(&init_user_ns, dentry, iap, NULL); out_unlock: - fh_unlock(fhp); + inode_unlock(inode); if (size_change) put_write_access(inode); out: @@ -2196,12 +2196,8 @@ nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char **bufp, } /* - * Removexattr and setxattr need to call fh_lock to both lock the inode - * and set the change attribute. Since the top-level vfs_removexattr - * and vfs_setxattr calls already do their own inode_lock calls, call - * the _locked variant. Pass in a NULL pointer for delegated_inode, - * and let the client deal with NFS4ERR_DELAY (same as with e.g. - * setattr and remove). + * Pass in a NULL pointer for delegated_inode, and let the client deal + * with NFS4ERR_DELAY (same as with e.g. setattr and remove). */ __be32 nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name) @@ -2217,12 +2213,14 @@ nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name) if (ret) return nfserrno(ret); - fh_lock(fhp); + inode_lock(fhp->fh_dentry->d_inode); + fh_fill_pre_attrs(fhp); ret = __vfs_removexattr_locked(&init_user_ns, fhp->fh_dentry, name, NULL); - fh_unlock(fhp); + fh_fill_post_attrs(fhp); + inode_unlock(fhp->fh_dentry->d_inode); fh_drop_write(fhp); return nfsd_xattr_errno(ret); @@ -2242,12 +2240,13 @@ nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name, ret = fh_want_write(fhp); if (ret) return nfserrno(ret); - fh_lock(fhp); + inode_lock(fhp->fh_dentry->d_inode); + fh_fill_pre_attrs(fhp); ret = __vfs_setxattr_locked(&init_user_ns, fhp->fh_dentry, name, buf, len, flags, NULL); - - fh_unlock(fhp); + fh_fill_post_attrs(fhp); + inode_unlock(fhp->fh_dentry->d_inode); fh_drop_write(fhp); return nfsd_xattr_errno(ret); From patchwork Wed Jul 6 04:18:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 12907316 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 96296C433EF for ; Wed, 6 Jul 2022 04:21:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231210AbiGFEVh (ORCPT ); Wed, 6 Jul 2022 00:21:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43588 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231777AbiGFEVg (ORCPT ); Wed, 6 Jul 2022 00:21:36 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 129BA17A9D for ; Tue, 5 Jul 2022 21:21:34 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id B14B41F8D8; Wed, 6 Jul 2022 04:21:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657081292; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/3ntBbPLLmlhlDCgeuZPn3uytaoNBzdWQ/MVtf7ajeo=; b=niwKeeKJVq0A5PO2jZsK3GCJEwy7ZC+fO+16McG88UBxw3BPgkvpIsylIe2ByqYc/BZvkM OI2S57GeU9q0njopslKBQ0yY2fm6iNfOOKF0uO/CT+P3QysZzVSHQExTb7HNvhg3PttGhx lGzVEE6JyK/eJAj9uNZr2RVWI1C32aA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657081292; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/3ntBbPLLmlhlDCgeuZPn3uytaoNBzdWQ/MVtf7ajeo=; b=plQt2mN9HQvVaE/Xuv7QCb7Fh/ibPPYlLUshsRHMJkua9rgn1IkU5w4lxNK/ZsGhZh6uLD ohvBbRsyi18prbDg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 8F9DD13A37; Wed, 6 Jul 2022 04:21:31 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id GMl/EssNxWIsQgAAMHmgww (envelope-from ); Wed, 06 Jul 2022 04:21:31 +0000 Subject: [PATCH 8/8] NFSD: discard fh_locked flag and fh_lock/fh_unlock From: NeilBrown To: Chuck Lever , Jeff Layton Cc: linux-nfs@vger.kernel.org Date: Wed, 06 Jul 2022 14:18:12 +1000 Message-ID: <165708109261.1940.5366273190007170909.stgit@noble.brown> In-Reply-To: <165708033167.1940.3364591321728458949.stgit@noble.brown> References: <165708033167.1940.3364591321728458949.stgit@noble.brown> User-Agent: StGit/1.5 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org As all inode locking is now fully balanced, fh_put() does not need to call fh_unlock(). fh_lock() and fh_unlock() are no longer used, so discard them. These are the only real users of ->fh_locked, so discard that too. Signed-off-by: NeilBrown Reviewed-by: Jeff Layton --- fs/nfsd/nfsfh.c | 3 +-- fs/nfsd/nfsfh.h | 56 ++++--------------------------------------------------- fs/nfsd/vfs.c | 17 +---------------- 3 files changed, 6 insertions(+), 70 deletions(-) diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c index 5e2ed4b2a925..22a77a5e2327 100644 --- a/fs/nfsd/nfsfh.c +++ b/fs/nfsd/nfsfh.c @@ -549,7 +549,7 @@ fh_compose(struct svc_fh *fhp, struct svc_export *exp, struct dentry *dentry, if (ref_fh == fhp) fh_put(ref_fh); - if (fhp->fh_locked || fhp->fh_dentry) { + if (fhp->fh_dentry) { printk(KERN_ERR "fh_compose: fh %pd2 not initialized!\n", dentry); } @@ -681,7 +681,6 @@ fh_put(struct svc_fh *fhp) struct dentry * dentry = fhp->fh_dentry; struct svc_export * exp = fhp->fh_export; if (dentry) { - fh_unlock(fhp); fhp->fh_dentry = NULL; dput(dentry); fh_clear_pre_post_attrs(fhp); diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h index fb9d358a267e..09c654bdf9b0 100644 --- a/fs/nfsd/nfsfh.h +++ b/fs/nfsd/nfsfh.h @@ -81,7 +81,6 @@ typedef struct svc_fh { struct dentry * fh_dentry; /* validated dentry */ struct svc_export * fh_export; /* export pointer */ - bool fh_locked; /* inode locked by us */ bool fh_want_write; /* remount protection taken */ bool fh_no_wcc; /* no wcc data needed */ bool fh_no_atomic_attr; @@ -93,7 +92,7 @@ typedef struct svc_fh { bool fh_post_saved; /* post-op attrs saved */ bool fh_pre_saved; /* pre-op attrs saved */ - /* Pre-op attributes saved during fh_lock */ + /* Pre-op attributes saved when inode is locked */ __u64 fh_pre_size; /* size before operation */ struct timespec64 fh_pre_mtime; /* mtime before oper */ struct timespec64 fh_pre_ctime; /* ctime before oper */ @@ -103,7 +102,7 @@ typedef struct svc_fh { */ u64 fh_pre_change; - /* Post-op attributes saved in fh_unlock */ + /* Post-op attributes saved in fh_fill_post_attrs() */ struct kstat fh_post_attr; /* full attrs after operation */ u64 fh_post_change; /* nfsv4 change; see above */ } svc_fh; @@ -223,8 +222,8 @@ void fh_put(struct svc_fh *); static __inline__ struct svc_fh * fh_copy(struct svc_fh *dst, struct svc_fh *src) { - WARN_ON(src->fh_dentry || src->fh_locked); - + WARN_ON(src->fh_dentry); + *dst = *src; return dst; } @@ -323,51 +322,4 @@ static inline u64 nfsd4_change_attribute(struct kstat *stat, extern void fh_fill_pre_attrs(struct svc_fh *fhp); extern void fh_fill_post_attrs(struct svc_fh *fhp); - -/* - * Lock a file handle/inode - * NOTE: both fh_lock and fh_unlock are done "by hand" in - * vfs.c:nfsd_rename as it needs to grab 2 i_mutex's at once - * so, any changes here should be reflected there. - */ - -static inline void -fh_lock_nested(struct svc_fh *fhp, unsigned int subclass) -{ - struct dentry *dentry = fhp->fh_dentry; - struct inode *inode; - - BUG_ON(!dentry); - - if (fhp->fh_locked) { - printk(KERN_WARNING "fh_lock: %pd2 already locked!\n", - dentry); - return; - } - - inode = d_inode(dentry); - inode_lock_nested(inode, subclass); - fh_fill_pre_attrs(fhp); - fhp->fh_locked = true; -} - -static inline void -fh_lock(struct svc_fh *fhp) -{ - fh_lock_nested(fhp, I_MUTEX_NORMAL); -} - -/* - * Unlock a file handle/inode - */ -static inline void -fh_unlock(struct svc_fh *fhp) -{ - if (fhp->fh_locked) { - fh_fill_post_attrs(fhp); - inode_unlock(d_inode(fhp->fh_dentry)); - fhp->fh_locked = false; - } -} - #endif /* _LINUX_NFSD_NFSFH_H */ diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 2526615285ca..fe4cdf8ab428 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1305,13 +1305,6 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp, dirp = d_inode(dentry); dchild = dget(resfhp->fh_dentry); - if (!fhp->fh_locked) { - WARN_ONCE(1, "nfsd_create: parent %pd2 not locked!\n", - dentry); - err = nfserr_io; - goto out; - } - err = nfsd_permission(rqstp, fhp->fh_export, dentry, NFSD_MAY_CREATE); if (err) goto out; @@ -1674,10 +1667,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen, goto out; } - /* cannot use fh_lock as we need deadlock protective ordering - * so do it by hand */ trap = lock_rename(tdentry, fdentry); - ffhp->fh_locked = tfhp->fh_locked = true; fh_fill_pre_attrs(ffhp); fh_fill_pre_attrs(tfhp); @@ -1733,17 +1723,12 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen, dput(odentry); out_nfserr: err = nfserrno(host_err); - /* - * We cannot rely on fh_unlock on the two filehandles, - * as that would do the wrong thing if the two directories - * were the same, so again we do it by hand. - */ + if (!close_cached) { fh_fill_post_attrs(ffhp); fh_fill_post_attrs(tfhp); } unlock_rename(tdentry, fdentry); - ffhp->fh_locked = tfhp->fh_locked = false; fh_drop_write(ffhp); /*