From 50f5fdaea9cd6941e51ea6e07d8c15265917bb86 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Wed, 7 Dec 2022 09:43:07 +0100 Subject: NFSD: Teach nfsd_mountpoint() auto mounts Currently nfsd_mountpoint() tests for mount points using d_mountpoint(), this works only when a mount point is already uncovered. In our case the mount point is of type auto mount and can be coverted. i.e. ->d_automount() was not called. Using d_managed() nfsd_mountpoint() can test whether a mount point is either already uncovered or can be uncovered later. Signed-off-by: Richard Weinberger Reviewed-by: Ian Kent Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/vfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 4c3a0d84043c..780856561bbb 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -223,7 +223,7 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp) return 1; if (nfsd4_is_junction(dentry)) return 1; - if (d_mountpoint(dentry)) + if (d_managed(dentry)) /* * Might only be a mountpoint in a different namespace, * but we need to check. -- cgit v1.2.3-70-g09d2 From e1f19857f94be09f9526f180e64f20138bd4e394 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Wed, 7 Dec 2022 09:43:08 +0100 Subject: fs: namei: Allow follow_down() to uncover auto mounts This function is only used by NFSD to cross mount points. If a mount point is of type auto mount, follow_down() will not uncover it. Add LOOKUP_AUTOMOUNT to the lookup flags to have ->d_automount() called when NFSD walks down the mount tree. Signed-off-by: Richard Weinberger Reviewed-by: Ian Kent Reviewed-by: Jeff Layton Acked-by: Al Viro Signed-off-by: Chuck Lever --- fs/namei.c | 6 +++--- fs/nfsd/vfs.c | 6 +++++- include/linux/namei.h | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/namei.c b/fs/namei.c index 309ae6fc8c99..c06f47066063 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1458,11 +1458,11 @@ EXPORT_SYMBOL(follow_down_one); * point, the filesystem owning that dentry may be queried as to whether the * caller is permitted to proceed or not. */ -int follow_down(struct path *path) +int follow_down(struct path *path, unsigned int flags) { struct vfsmount *mnt = path->mnt; bool jumped; - int ret = traverse_mounts(path, &jumped, NULL, 0); + int ret = traverse_mounts(path, &jumped, NULL, flags); if (path->mnt != mnt) mntput(mnt); @@ -2864,7 +2864,7 @@ int path_pts(struct path *path) path->dentry = child; dput(parent); - follow_down(path); + follow_down(path, 0); return 0; } #endif diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 780856561bbb..21d5209f6e04 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -126,9 +126,13 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp, struct dentry *dentry = *dpp; struct path path = {.mnt = mntget(exp->ex_path.mnt), .dentry = dget(dentry)}; + unsigned int follow_flags = 0; int err = 0; - err = follow_down(&path); + if (exp->ex_flags & NFSEXP_CROSSMOUNT) + follow_flags = LOOKUP_AUTOMOUNT; + + err = follow_down(&path, follow_flags); if (err < 0) goto out; if (path.mnt == exp->ex_path.mnt && path.dentry == dentry && diff --git a/include/linux/namei.h b/include/linux/namei.h index 00fee52df842..c82e2ac65846 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -77,7 +77,7 @@ struct dentry *lookup_one_positive_unlocked(struct user_namespace *mnt_userns, struct dentry *base, int len); extern int follow_down_one(struct path *); -extern int follow_down(struct path *); +extern int follow_down(struct path *path, unsigned int flags); extern int follow_up(struct path *); extern struct dentry *lock_rename(struct dentry *, struct dentry *); -- cgit v1.2.3-70-g09d2 From f78e44545814b26ab6af7cdd5b022293ceac867e Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Wed, 7 Dec 2022 09:43:09 +0100 Subject: NFS: nfs_encode_fh: Remove S_AUTOMOUNT check Now with NFSD being able to cross into auto mounts, the check can be removed. Signed-off-by: Richard Weinberger Reviewed-by: Ian Kent Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever Acked-by: Anna Schumaker --- fs/nfs/export.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/nfs/export.c b/fs/nfs/export.c index 01596f2d0a1e..0a5ee1754d50 100644 --- a/fs/nfs/export.c +++ b/fs/nfs/export.c @@ -42,7 +42,7 @@ nfs_encode_fh(struct inode *inode, __u32 *p, int *max_len, struct inode *parent) dprintk("%s: max fh len %d inode %p parent %p", __func__, *max_len, inode, parent); - if (*max_len < len || IS_AUTOMOUNT(inode)) { + if (*max_len < len) { dprintk("%s: fh len %d too small, required %d\n", __func__, *max_len, len); *max_len = len; -- cgit v1.2.3-70-g09d2 From dba5eaa46b0282cb9607d362c8887dfcb44bfd2e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 2 Jan 2023 12:05:30 -0500 Subject: SUNRPC: Push svcxdr_init_decode() into svc_process_common() Now that all vs_dispatch functions invoke svcxdr_init_decode(), it is common code and can be pushed down into the generic RPC server. Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/lockd/svc.c | 1 - fs/nfs/callback_xdr.c | 1 - fs/nfsd/nfssvc.c | 1 - net/sunrpc/svc.c | 1 + 4 files changed, 1 insertion(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index 59ef8a1f843f..e56d85335599 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -695,7 +695,6 @@ static int nlmsvc_dispatch(struct svc_rqst *rqstp, __be32 *statp) { const struct svc_procedure *procp = rqstp->rq_procinfo; - svcxdr_init_decode(rqstp); if (!procp->pc_decode(rqstp, &rqstp->rq_arg_stream)) goto out_decode_err; diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c index d0cccddb7d08..46d3f5986b4e 100644 --- a/fs/nfs/callback_xdr.c +++ b/fs/nfs/callback_xdr.c @@ -984,7 +984,6 @@ nfs_callback_dispatch(struct svc_rqst *rqstp, __be32 *statp) { const struct svc_procedure *procp = rqstp->rq_procinfo; - svcxdr_init_decode(rqstp); svcxdr_init_encode(rqstp); *statp = procp->pc_func(rqstp); diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index 325d3d3f1211..1ed29eac80ed 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -1040,7 +1040,6 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp) */ rqstp->rq_cachetype = proc->pc_cachetype; - svcxdr_init_decode(rqstp); if (!proc->pc_decode(rqstp, &rqstp->rq_arg_stream)) goto out_decode_err; diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index f06622814a95..8d1700e2ce1d 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -1302,6 +1302,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *argv, struct kvec *resv) if (progp == NULL) goto err_bad_prog; + svcxdr_init_decode(rqstp); rpc_stat = progp->pg_init_request(rqstp, progp, &process); switch (rpc_stat) { case rpc_success: -- cgit v1.2.3-70-g09d2 From df24ac7a2e3a9d0bc68f1756a880e50bfe4b4522 Mon Sep 17 00:00:00 2001 From: Dai Ngo Date: Sun, 18 Dec 2022 16:55:53 -0800 Subject: NFSD: enhance inter-server copy cleanup Currently nfsd4_setup_inter_ssc returns the vfsmount of the source server's export when the mount completes. After the copy is done nfsd4_cleanup_inter_ssc is called with the vfsmount of the source server and it searches nfsd_ssc_mount_list for a matching entry to do the clean up. The problems with this approach are (1) the need to search the nfsd_ssc_mount_list and (2) the code has to handle the case where the matching entry is not found which looks ugly. The enhancement is instead of nfsd4_setup_inter_ssc returning the vfsmount, it returns the nfsd4_ssc_umount_item which has the vfsmount embedded in it. When nfsd4_cleanup_inter_ssc is called it's passed with the nfsd4_ssc_umount_item directly to do the clean up so no searching is needed and there is no need to handle the 'not found' case. Signed-off-by: Dai Ngo Signed-off-by: Chuck Lever [ cel: adjusted whitespace and variable/function names ] Reviewed-by: Olga Kornievskaia --- fs/nfsd/nfs4proc.c | 111 +++++++++++++++++++----------------------------- fs/nfsd/xdr4.h | 2 +- include/linux/nfs_ssc.h | 2 +- 3 files changed, 46 insertions(+), 69 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index f189ba7995f5..dbaf33398c82 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1293,15 +1293,15 @@ extern void nfs_sb_deactive(struct super_block *sb); * setup a work entry in the ssc delayed unmount list. */ static __be32 nfsd4_ssc_setup_dul(struct nfsd_net *nn, char *ipaddr, - struct nfsd4_ssc_umount_item **retwork, struct vfsmount **ss_mnt) + struct nfsd4_ssc_umount_item **nsui) { struct nfsd4_ssc_umount_item *ni = NULL; struct nfsd4_ssc_umount_item *work = NULL; struct nfsd4_ssc_umount_item *tmp; DEFINE_WAIT(wait); + __be32 status = 0; - *ss_mnt = NULL; - *retwork = NULL; + *nsui = NULL; work = kzalloc(sizeof(*work), GFP_KERNEL); try_again: spin_lock(&nn->nfsd_ssc_lock); @@ -1325,12 +1325,12 @@ try_again: finish_wait(&nn->nfsd_ssc_waitq, &wait); goto try_again; } - *ss_mnt = ni->nsui_vfsmount; + *nsui = ni; refcount_inc(&ni->nsui_refcnt); spin_unlock(&nn->nfsd_ssc_lock); kfree(work); - /* return vfsmount in ss_mnt */ + /* return vfsmount in (*nsui)->nsui_vfsmount */ return 0; } if (work) { @@ -1338,31 +1338,32 @@ try_again: refcount_set(&work->nsui_refcnt, 2); work->nsui_busy = true; list_add_tail(&work->nsui_list, &nn->nfsd_ssc_mount_list); - *retwork = work; - } + *nsui = work; + } else + status = nfserr_resource; spin_unlock(&nn->nfsd_ssc_lock); - return 0; + return status; } -static void nfsd4_ssc_update_dul_work(struct nfsd_net *nn, - struct nfsd4_ssc_umount_item *work, struct vfsmount *ss_mnt) +static void nfsd4_ssc_update_dul(struct nfsd_net *nn, + struct nfsd4_ssc_umount_item *nsui, + struct vfsmount *ss_mnt) { - /* set nsui_vfsmount, clear busy flag and wakeup waiters */ spin_lock(&nn->nfsd_ssc_lock); - work->nsui_vfsmount = ss_mnt; - work->nsui_busy = false; + nsui->nsui_vfsmount = ss_mnt; + nsui->nsui_busy = false; wake_up_all(&nn->nfsd_ssc_waitq); spin_unlock(&nn->nfsd_ssc_lock); } -static void nfsd4_ssc_cancel_dul_work(struct nfsd_net *nn, - struct nfsd4_ssc_umount_item *work) +static void nfsd4_ssc_cancel_dul(struct nfsd_net *nn, + struct nfsd4_ssc_umount_item *nsui) { spin_lock(&nn->nfsd_ssc_lock); - list_del(&work->nsui_list); + list_del(&nsui->nsui_list); wake_up_all(&nn->nfsd_ssc_waitq); spin_unlock(&nn->nfsd_ssc_lock); - kfree(work); + kfree(nsui); } /* @@ -1370,7 +1371,7 @@ static void nfsd4_ssc_cancel_dul_work(struct nfsd_net *nn, */ static __be32 nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, - struct vfsmount **mount) + struct nfsd4_ssc_umount_item **nsui) { struct file_system_type *type; struct vfsmount *ss_mnt; @@ -1381,7 +1382,6 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, char *ipaddr, *dev_name, *raw_data; int len, raw_len; __be32 status = nfserr_inval; - struct nfsd4_ssc_umount_item *work = NULL; struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id); naddr = &nss->u.nl4_addr; @@ -1389,6 +1389,7 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, naddr->addr_len, (struct sockaddr *)&tmp_addr, sizeof(tmp_addr)); + *nsui = NULL; if (tmp_addrlen == 0) goto out_err; @@ -1431,10 +1432,10 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, goto out_free_rawdata; snprintf(dev_name, len + 5, "%s%s%s:/", startsep, ipaddr, endsep); - status = nfsd4_ssc_setup_dul(nn, ipaddr, &work, &ss_mnt); + status = nfsd4_ssc_setup_dul(nn, ipaddr, nsui); if (status) goto out_free_devname; - if (ss_mnt) + if ((*nsui)->nsui_vfsmount) goto out_done; /* Use an 'internal' mount: SB_KERNMOUNT -> MNT_INTERNAL */ @@ -1442,15 +1443,12 @@ nfsd4_interssc_connect(struct nl4_server *nss, struct svc_rqst *rqstp, module_put(type->owner); if (IS_ERR(ss_mnt)) { status = nfserr_nodev; - if (work) - nfsd4_ssc_cancel_dul_work(nn, work); + nfsd4_ssc_cancel_dul(nn, *nsui); goto out_free_devname; } - if (work) - nfsd4_ssc_update_dul_work(nn, work, ss_mnt); + nfsd4_ssc_update_dul(nn, *nsui, ss_mnt); out_done: status = 0; - *mount = ss_mnt; out_free_devname: kfree(dev_name); @@ -1474,7 +1472,7 @@ out_err: static __be32 nfsd4_setup_inter_ssc(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, - struct nfsd4_copy *copy, struct vfsmount **mount) + struct nfsd4_copy *copy) { struct svc_fh *s_fh = NULL; stateid_t *s_stid = ©->cp_src_stateid; @@ -1487,7 +1485,7 @@ nfsd4_setup_inter_ssc(struct svc_rqst *rqstp, if (status) goto out; - status = nfsd4_interssc_connect(copy->cp_src, rqstp, mount); + status = nfsd4_interssc_connect(copy->cp_src, rqstp, ©->ss_nsui); if (status) goto out; @@ -1505,45 +1503,27 @@ out: } static void -nfsd4_cleanup_inter_ssc(struct vfsmount *ss_mnt, struct file *filp, +nfsd4_cleanup_inter_ssc(struct nfsd4_ssc_umount_item *nsui, struct file *filp, struct nfsd_file *dst) { - bool found = false; - long timeout; - struct nfsd4_ssc_umount_item *tmp; - struct nfsd4_ssc_umount_item *ni = NULL; struct nfsd_net *nn = net_generic(dst->nf_net, nfsd_net_id); + long timeout = msecs_to_jiffies(nfsd4_ssc_umount_timeout); nfs42_ssc_close(filp); nfsd_file_put(dst); fput(filp); - if (!nn) { - mntput(ss_mnt); - return; - } spin_lock(&nn->nfsd_ssc_lock); - timeout = msecs_to_jiffies(nfsd4_ssc_umount_timeout); - list_for_each_entry_safe(ni, tmp, &nn->nfsd_ssc_mount_list, nsui_list) { - if (ni->nsui_vfsmount->mnt_sb == ss_mnt->mnt_sb) { - list_del(&ni->nsui_list); - /* - * vfsmount can be shared by multiple exports, - * decrement refcnt. If the count drops to 1 it - * will be unmounted when nsui_expire expires. - */ - refcount_dec(&ni->nsui_refcnt); - ni->nsui_expire = jiffies + timeout; - list_add_tail(&ni->nsui_list, &nn->nfsd_ssc_mount_list); - found = true; - break; - } - } + list_del(&nsui->nsui_list); + /* + * vfsmount can be shared by multiple exports, + * decrement refcnt. If the count drops to 1 it + * will be unmounted when nsui_expire expires. + */ + refcount_dec(&nsui->nsui_refcnt); + nsui->nsui_expire = jiffies + timeout; + list_add_tail(&nsui->nsui_list, &nn->nfsd_ssc_mount_list); spin_unlock(&nn->nfsd_ssc_lock); - if (!found) { - mntput(ss_mnt); - return; - } } #else /* CONFIG_NFSD_V4_2_INTER_SSC */ @@ -1551,15 +1531,13 @@ nfsd4_cleanup_inter_ssc(struct vfsmount *ss_mnt, struct file *filp, static __be32 nfsd4_setup_inter_ssc(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, - struct nfsd4_copy *copy, - struct vfsmount **mount) + struct nfsd4_copy *copy) { - *mount = NULL; return nfserr_inval; } static void -nfsd4_cleanup_inter_ssc(struct vfsmount *ss_mnt, struct file *filp, +nfsd4_cleanup_inter_ssc(struct nfsd4_ssc_umount_item *nsui, struct file *filp, struct nfsd_file *dst) { } @@ -1700,7 +1678,7 @@ static void dup_copy_fields(struct nfsd4_copy *src, struct nfsd4_copy *dst) memcpy(dst->cp_src, src->cp_src, sizeof(struct nl4_server)); memcpy(&dst->stateid, &src->stateid, sizeof(src->stateid)); memcpy(&dst->c_fh, &src->c_fh, sizeof(src->c_fh)); - dst->ss_mnt = src->ss_mnt; + dst->ss_nsui = src->ss_nsui; } static void cleanup_async_copy(struct nfsd4_copy *copy) @@ -1749,8 +1727,8 @@ static int nfsd4_do_async_copy(void *data) if (nfsd4_ssc_is_inter(copy)) { struct file *filp; - filp = nfs42_ssc_open(copy->ss_mnt, ©->c_fh, - ©->stateid); + filp = nfs42_ssc_open(copy->ss_nsui->nsui_vfsmount, + ©->c_fh, ©->stateid); if (IS_ERR(filp)) { switch (PTR_ERR(filp)) { case -EBADF: @@ -1764,7 +1742,7 @@ static int nfsd4_do_async_copy(void *data) } nfserr = nfsd4_do_copy(copy, filp, copy->nf_dst->nf_file, false); - nfsd4_cleanup_inter_ssc(copy->ss_mnt, filp, copy->nf_dst); + nfsd4_cleanup_inter_ssc(copy->ss_nsui, filp, copy->nf_dst); } else { nfserr = nfsd4_do_copy(copy, copy->nf_src->nf_file, copy->nf_dst->nf_file, false); @@ -1790,8 +1768,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, status = nfserr_notsupp; goto out; } - status = nfsd4_setup_inter_ssc(rqstp, cstate, copy, - ©->ss_mnt); + status = nfsd4_setup_inter_ssc(rqstp, cstate, copy); if (status) return nfserr_offload_denied; } else { diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h index 4fd2cf6d1d2d..510978e602da 100644 --- a/fs/nfsd/xdr4.h +++ b/fs/nfsd/xdr4.h @@ -571,7 +571,7 @@ struct nfsd4_copy { struct task_struct *copy_task; refcount_t refcount; - struct vfsmount *ss_mnt; + struct nfsd4_ssc_umount_item *ss_nsui; struct nfs_fh c_fh; nfs4_stateid stateid; }; diff --git a/include/linux/nfs_ssc.h b/include/linux/nfs_ssc.h index 75843c00f326..22265b1ff080 100644 --- a/include/linux/nfs_ssc.h +++ b/include/linux/nfs_ssc.h @@ -53,6 +53,7 @@ static inline void nfs42_ssc_close(struct file *filep) if (nfs_ssc_client_tbl.ssc_nfs4_ops) (*nfs_ssc_client_tbl.ssc_nfs4_ops->sco_close)(filep); } +#endif struct nfsd4_ssc_umount_item { struct list_head nsui_list; @@ -66,7 +67,6 @@ struct nfsd4_ssc_umount_item { struct vfsmount *nsui_vfsmount; char nsui_ipaddr[RPC_MAX_ADDRBUFLEN + 1]; }; -#endif /* * NFS_FS -- cgit v1.2.3-70-g09d2 From 70f62231cdfd52357836733dd31db787e0412ab2 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 6 Jan 2023 10:33:47 -0500 Subject: nfsd: allow nfsd_file_get to sanely handle a NULL pointer ...and remove some now-useless NULL pointer checks in its callers. Suggested-by: NeilBrown Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/filecache.c | 5 ++--- fs/nfsd/nfs4state.c | 4 +--- 2 files changed, 3 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c index c0950edb26b0..ecc32105c3ab 100644 --- a/fs/nfsd/filecache.c +++ b/fs/nfsd/filecache.c @@ -452,7 +452,7 @@ static bool nfsd_file_lru_remove(struct nfsd_file *nf) struct nfsd_file * nfsd_file_get(struct nfsd_file *nf) { - if (likely(refcount_inc_not_zero(&nf->nf_ref))) + if (nf && refcount_inc_not_zero(&nf->nf_ref)) return nf; return NULL; } @@ -1107,8 +1107,7 @@ retry: rcu_read_lock(); nf = rhashtable_lookup(&nfsd_file_rhash_tbl, &key, nfsd_file_rhash_params); - if (nf) - nf = nfsd_file_get(nf); + nf = nfsd_file_get(nf); rcu_read_unlock(); if (nf) { diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index c69f27d3adb7..9657dab980cb 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -602,9 +602,7 @@ put_nfs4_file(struct nfs4_file *fi) static struct nfsd_file * __nfs4_get_fd(struct nfs4_file *f, int oflag) { - if (f->fi_fds[oflag]) - return nfsd_file_get(f->fi_fds[oflag]); - return NULL; + return nfsd_file_get(f->fi_fds[oflag]); } static struct nfsd_file * -- cgit v1.2.3-70-g09d2 From bd6aaf781dae436727928cce514881d3c32758b9 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 6 Jan 2023 10:33:48 -0500 Subject: nfsd: fix potential race in nfs4_find_file The WARN_ON_ONCE check is not terribly useful. It also seems possible for nfs4_find_file to race with the destruction of an fi_deleg_file while trying to take a reference to it. Now that it's safe to pass nfs_get_file a NULL pointer, remove the WARN and NULL pointer check. Take the fi_lock when fetching fi_deleg_file. Cc: NeilBrown Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4state.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 9657dab980cb..7ed723994591 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -6404,23 +6404,26 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate, static struct nfsd_file * nfs4_find_file(struct nfs4_stid *s, int flags) { + struct nfsd_file *ret = NULL; + if (!s) return NULL; switch (s->sc_type) { case NFS4_DELEG_STID: - if (WARN_ON_ONCE(!s->sc_file->fi_deleg_file)) - return NULL; - return nfsd_file_get(s->sc_file->fi_deleg_file); + spin_lock(&s->sc_file->fi_lock); + ret = nfsd_file_get(s->sc_file->fi_deleg_file); + spin_unlock(&s->sc_file->fi_lock); + break; case NFS4_OPEN_STID: case NFS4_LOCK_STID: if (flags & RD_STATE) - return find_readable_file(s->sc_file); + ret = find_readable_file(s->sc_file); else - return find_writeable_file(s->sc_file); + ret = find_writeable_file(s->sc_file); } - return NULL; + return ret; } static __be32 -- cgit v1.2.3-70-g09d2 From 8dd41d70f331c342842e8d349d7a1f73b0ba7ccd Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sun, 8 Jan 2023 11:29:44 -0500 Subject: SUNRPC: Push svcxdr_init_encode() into svc_process_common() Now that all vs_dispatch functions invoke svcxdr_init_encode(), it is common code and can be pushed down into the generic RPC server. Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/lockd/svc.c | 1 - fs/nfs/callback_xdr.c | 2 -- fs/nfsd/nfscache.c | 2 +- fs/nfsd/nfssvc.c | 6 ------ include/linux/sunrpc/xdr.h | 21 +++++++++++++++++++++ net/sunrpc/svc.c | 1 + 6 files changed, 23 insertions(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index e56d85335599..642e394e7a2d 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -704,7 +704,6 @@ static int nlmsvc_dispatch(struct svc_rqst *rqstp, __be32 *statp) if (*statp != rpc_success) return 1; - svcxdr_init_encode(rqstp); if (!procp->pc_encode(rqstp, &rqstp->rq_res_stream)) goto out_encode_err; diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c index 46d3f5986b4e..b4c3b7182198 100644 --- a/fs/nfs/callback_xdr.c +++ b/fs/nfs/callback_xdr.c @@ -984,8 +984,6 @@ nfs_callback_dispatch(struct svc_rqst *rqstp, __be32 *statp) { const struct svc_procedure *procp = rqstp->rq_procinfo; - svcxdr_init_encode(rqstp); - *statp = procp->pc_func(rqstp); return 1; } diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c index 3e64a3d50a1c..ef5ee548053b 100644 --- a/fs/nfsd/nfscache.c +++ b/fs/nfsd/nfscache.c @@ -488,7 +488,7 @@ found_entry: case RC_NOCACHE: break; case RC_REPLSTAT: - svc_putu32(&rqstp->rq_res.head[0], rp->c_replstat); + xdr_stream_encode_be32(&rqstp->rq_res_stream, rp->c_replstat); rtn = RC_REPLY; break; case RC_REPLBUFF: diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index 1ed29eac80ed..dfa8ee6c04d5 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -1052,12 +1052,6 @@ int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp) goto out_dropit; } - /* - * Need to grab the location to store the status, as - * NFSv4 does some encoding while processing - */ - svcxdr_init_encode(rqstp); - *statp = proc->pc_func(rqstp); if (test_bit(RQ_DROPME, &rqstp->rq_flags)) goto out_update_drop; diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h index f3b6eb9accd7..72014c9216fc 100644 --- a/include/linux/sunrpc/xdr.h +++ b/include/linux/sunrpc/xdr.h @@ -474,6 +474,27 @@ xdr_stream_encode_u32(struct xdr_stream *xdr, __u32 n) return len; } +/** + * xdr_stream_encode_be32 - Encode a big-endian 32-bit integer + * @xdr: pointer to xdr_stream + * @n: integer to encode + * + * Return values: + * On success, returns length in bytes of XDR buffer consumed + * %-EMSGSIZE on XDR buffer overflow + */ +static inline ssize_t +xdr_stream_encode_be32(struct xdr_stream *xdr, __be32 n) +{ + const size_t len = sizeof(n); + __be32 *p = xdr_reserve_space(xdr, len); + + if (unlikely(!p)) + return -EMSGSIZE; + *p = n; + return len; +} + /** * xdr_stream_encode_u64 - Encode a 64-bit integer * @xdr: pointer to xdr_stream diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 7610e36e51e0..ce75db5eb9bd 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -1321,6 +1321,7 @@ svc_process_common(struct svc_rqst *rqstp, struct kvec *resv) */ if (procp->pc_xdrressize) svc_reserve_auth(rqstp, procp->pc_xdrressize<<2); + svcxdr_init_encode(rqstp); /* Call the function that processes the request. */ rc = process.dispatch(rqstp, statp); -- cgit v1.2.3-70-g09d2 From cee4db19452467eef8ab93c6eb6a3a84d11d25d7 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sun, 8 Jan 2023 11:30:59 -0500 Subject: SUNRPC: Refactor RPC server dispatch method Currently, svcauth_gss_accept() pre-reserves response buffer space for the RPC payload length and GSS sequence number before returning to the dispatcher, which then adds the header's accept_stat field. The problem is the accept_stat field is supposed to go before the length and seq_num fields. So svcauth_gss_release() has to relocate the accept_stat value (see svcauth_gss_prepare_to_wrap()). To enable these fields to be added to the response buffer in the correct (final) order, the pointer to the accept_stat has to be made available to svcauth_gss_accept() so that it can set it before reserving space for the length and seq_num fields. As a first step, move the pointer to the location of the accept_stat field into struct svc_rqst. Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/lockd/svc.c | 4 ++-- fs/nfs/callback_xdr.c | 4 ++-- fs/nfsd/nfscache.c | 2 +- fs/nfsd/nfsd.h | 2 +- fs/nfsd/nfssvc.c | 4 ++-- include/linux/sunrpc/svc.h | 5 +++-- net/sunrpc/svc.c | 10 +++++----- 7 files changed, 16 insertions(+), 15 deletions(-) (limited to 'fs') diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index 642e394e7a2d..0b28a6cf9303 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -685,15 +685,15 @@ module_exit(exit_nlm); /** * nlmsvc_dispatch - Process an NLM Request * @rqstp: incoming request - * @statp: pointer to location of accept_stat field in RPC Reply buffer * * Return values: * %0: Processing complete; do not send a Reply * %1: Processing complete; send Reply in rqstp->rq_res */ -static int nlmsvc_dispatch(struct svc_rqst *rqstp, __be32 *statp) +static int nlmsvc_dispatch(struct svc_rqst *rqstp) { const struct svc_procedure *procp = rqstp->rq_procinfo; + __be32 *statp = rqstp->rq_accept_statp; if (!procp->pc_decode(rqstp, &rqstp->rq_arg_stream)) goto out_decode_err; diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c index b4c3b7182198..13b2af55497d 100644 --- a/fs/nfs/callback_xdr.c +++ b/fs/nfs/callback_xdr.c @@ -980,11 +980,11 @@ out_invalidcred: } static int -nfs_callback_dispatch(struct svc_rqst *rqstp, __be32 *statp) +nfs_callback_dispatch(struct svc_rqst *rqstp) { const struct svc_procedure *procp = rqstp->rq_procinfo; - *statp = procp->pc_func(rqstp); + *rqstp->rq_accept_statp = procp->pc_func(rqstp); return 1; } diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c index ef5ee548053b..041faa13b852 100644 --- a/fs/nfsd/nfscache.c +++ b/fs/nfsd/nfscache.c @@ -509,7 +509,7 @@ out_trace: * nfsd_cache_update - Update an entry in the duplicate reply cache. * @rqstp: svc_rqst with a finished Reply * @cachetype: which cache to update - * @statp: Reply's status code + * @statp: pointer to Reply's NFS status code, or NULL * * This is called from nfsd_dispatch when the procedure has been * executed and the complete reply is in rqstp->rq_res. diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h index fa0144a74267..d88498f8b275 100644 --- a/fs/nfsd/nfsd.h +++ b/fs/nfsd/nfsd.h @@ -86,7 +86,7 @@ bool nfssvc_encode_voidres(struct svc_rqst *rqstp, * Function prototypes. */ int nfsd_svc(int nrservs, struct net *net, const struct cred *cred); -int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp); +int nfsd_dispatch(struct svc_rqst *rqstp); int nfsd_nrthreads(struct net *); int nfsd_nrpools(struct net *); diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index dfa8ee6c04d5..ff10c46b62d3 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -1022,7 +1022,6 @@ out: /** * nfsd_dispatch - Process an NFS or NFSACL Request * @rqstp: incoming request - * @statp: pointer to location of accept_stat field in RPC Reply buffer * * This RPC dispatcher integrates the NFS server's duplicate reply cache. * @@ -1030,9 +1029,10 @@ out: * %0: Processing complete; do not send a Reply * %1: Processing complete; send Reply in rqstp->rq_res */ -int nfsd_dispatch(struct svc_rqst *rqstp, __be32 *statp) +int nfsd_dispatch(struct svc_rqst *rqstp) { const struct svc_procedure *proc = rqstp->rq_procinfo; + __be32 *statp = rqstp->rq_accept_statp; /* * Give the xdr decoder a chance to change this if it wants diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h index 32eb98e621c3..f40a90ca5de6 100644 --- a/include/linux/sunrpc/svc.h +++ b/include/linux/sunrpc/svc.h @@ -251,6 +251,7 @@ struct svc_rqst { void * rq_argp; /* decoded arguments */ void * rq_resp; /* xdr'd results */ + __be32 *rq_accept_statp; void * rq_auth_data; /* flavor-specific data */ __be32 rq_auth_stat; /* authentication status */ int rq_auth_slack; /* extra space xdr code @@ -337,7 +338,7 @@ struct svc_deferred_req { struct svc_process_info { union { - int (*dispatch)(struct svc_rqst *, __be32 *); + int (*dispatch)(struct svc_rqst *rqstp); struct { unsigned int lovers; unsigned int hivers; @@ -389,7 +390,7 @@ struct svc_version { bool vs_need_cong_ctrl; /* Dispatch function */ - int (*vs_dispatch)(struct svc_rqst *, __be32 *); + int (*vs_dispatch)(struct svc_rqst *rqstp); }; /* diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 167cb928515f..16760bc5191f 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -1232,9 +1232,9 @@ svc_process_common(struct svc_rqst *rqstp) const struct svc_procedure *procp = NULL; struct svc_serv *serv = rqstp->rq_server; struct svc_process_info process; - __be32 *p, *statp; int auth_res, rc; unsigned int aoffset; + __be32 *p; /* Will be turned off by GSS integrity and privacy services */ set_bit(RQ_SPLICE_OK, &rqstp->rq_flags); @@ -1314,8 +1314,8 @@ svc_process_common(struct svc_rqst *rqstp) trace_svc_process(rqstp, progp->pg_name); aoffset = xdr_stream_pos(xdr); - statp = xdr_reserve_space(&rqstp->rq_res_stream, XDR_UNIT); - *statp = rpc_success; + rqstp->rq_accept_statp = xdr_reserve_space(&rqstp->rq_res_stream, XDR_UNIT); + *rqstp->rq_accept_statp = rpc_success; /* un-reserve some of the out-queue now that we have a * better idea of reply size @@ -1324,7 +1324,7 @@ svc_process_common(struct svc_rqst *rqstp) svc_reserve_auth(rqstp, procp->pc_xdrressize<<2); /* Call the function that processes the request. */ - rc = process.dispatch(rqstp, statp); + rc = process.dispatch(rqstp); if (procp->pc_release) procp->pc_release(rqstp); if (!rc) @@ -1332,7 +1332,7 @@ svc_process_common(struct svc_rqst *rqstp) if (rqstp->rq_auth_stat != rpc_auth_ok) goto err_bad_auth; - if (*statp != rpc_success) + if (*rqstp->rq_accept_statp != rpc_success) xdr_truncate_encode(xdr, aoffset); if (procp->pc_encode == NULL) -- cgit v1.2.3-70-g09d2 From f5f9d4a314da88c0a5faa6d168bf69081b7a25ae Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 11 Jan 2023 11:19:59 -0500 Subject: nfsd: move reply cache initialization into nfsd startup There's no need to start the reply cache before nfsd is up and running, and doing so means that we register a shrinker for every net namespace instead of just the ones where nfsd is running. Move it to the per-net nfsd startup instead. Reported-by: Dai Ngo Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfsctl.c | 8 -------- fs/nfsd/nfssvc.c | 10 +++++++++- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index c2577ee7ffb2..f2a0d6ac88df 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1458,16 +1458,11 @@ static __net_init int nfsd_init_net(struct net *net) nn->nfsd_versions = NULL; nn->nfsd4_minorversions = NULL; nfsd4_init_leases_net(nn); - retval = nfsd_reply_cache_init(nn); - if (retval) - goto out_cache_error; get_random_bytes(&nn->siphash_key, sizeof(nn->siphash_key)); seqlock_init(&nn->writeverf_lock); return 0; -out_cache_error: - nfsd_idmap_shutdown(net); out_idmap_error: nfsd_export_shutdown(net); out_export_error: @@ -1476,9 +1471,6 @@ out_export_error: static __net_exit void nfsd_exit_net(struct net *net) { - struct nfsd_net *nn = net_generic(net, nfsd_net_id); - - nfsd_reply_cache_shutdown(nn); nfsd_idmap_shutdown(net); nfsd_export_shutdown(net); nfsd_netns_free_versions(net_generic(net, nfsd_net_id)); diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index ff10c46b62d3..fe5e4f73bb98 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -427,16 +427,23 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred) ret = nfsd_file_cache_start_net(net); if (ret) goto out_lockd; - ret = nfs4_state_start_net(net); + + ret = nfsd_reply_cache_init(nn); if (ret) goto out_filecache; + ret = nfs4_state_start_net(net); + if (ret) + goto out_reply_cache; + #ifdef CONFIG_NFSD_V4_2_INTER_SSC nfsd4_ssc_init_umount_work(nn); #endif nn->nfsd_net_up = true; return 0; +out_reply_cache: + nfsd_reply_cache_shutdown(nn); out_filecache: nfsd_file_cache_shutdown_net(net); out_lockd: @@ -454,6 +461,7 @@ static void nfsd_shutdown_net(struct net *net) struct nfsd_net *nn = net_generic(net, nfsd_net_id); nfs4_state_shutdown_net(net); + nfsd_reply_cache_shutdown(nn); nfsd_file_cache_shutdown_net(net); if (nn->lockd_up) { lockd_down(net); -- cgit v1.2.3-70-g09d2 From 65ba3d2425bf51165b6e88509c632bd15d12883d Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 10 Jan 2023 10:31:54 -0500 Subject: SUNRPC: Use per-CPU counters to tally server RPC counts - Improves counting accuracy - Reduces cross-CPU memory traffic Signed-off-by: Chuck Lever --- fs/lockd/svc.c | 15 ++++++++++----- fs/nfs/callback_xdr.c | 6 ++++-- fs/nfsd/nfs2acl.c | 5 +++-- fs/nfsd/nfs3acl.c | 5 +++-- fs/nfsd/nfs3proc.c | 5 +++-- fs/nfsd/nfs4proc.c | 7 ++++--- fs/nfsd/nfsproc.c | 6 +++--- include/linux/lockd/lockd.h | 4 ++-- include/linux/sunrpc/svc.h | 2 +- net/sunrpc/stats.c | 11 ++++++++--- net/sunrpc/svc.c | 2 +- 11 files changed, 42 insertions(+), 26 deletions(-) (limited to 'fs') diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index 0b28a6cf9303..1da00230860c 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -721,7 +721,7 @@ out_encode_err: /* * Define NLM program and procedures */ -static unsigned int nlmsvc_version1_count[17]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, nlmsvc_version1_count[17]); static const struct svc_version nlmsvc_version1 = { .vs_vers = 1, .vs_nproc = 17, @@ -730,26 +730,31 @@ static const struct svc_version nlmsvc_version1 = { .vs_dispatch = nlmsvc_dispatch, .vs_xdrsize = NLMSVC_XDRSIZE, }; -static unsigned int nlmsvc_version3_count[24]; + +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nlmsvc_version3_count[ARRAY_SIZE(nlmsvc_procedures)]); static const struct svc_version nlmsvc_version3 = { .vs_vers = 3, - .vs_nproc = 24, + .vs_nproc = ARRAY_SIZE(nlmsvc_procedures), .vs_proc = nlmsvc_procedures, .vs_count = nlmsvc_version3_count, .vs_dispatch = nlmsvc_dispatch, .vs_xdrsize = NLMSVC_XDRSIZE, }; + #ifdef CONFIG_LOCKD_V4 -static unsigned int nlmsvc_version4_count[24]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nlmsvc_version4_count[ARRAY_SIZE(nlmsvc_procedures4)]); static const struct svc_version nlmsvc_version4 = { .vs_vers = 4, - .vs_nproc = 24, + .vs_nproc = ARRAY_SIZE(nlmsvc_procedures4), .vs_proc = nlmsvc_procedures4, .vs_count = nlmsvc_version4_count, .vs_dispatch = nlmsvc_dispatch, .vs_xdrsize = NLMSVC_XDRSIZE, }; #endif + static const struct svc_version *nlmsvc_version[] = { [1] = &nlmsvc_version1, [3] = &nlmsvc_version3, diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c index 13b2af55497d..321af81c456e 100644 --- a/fs/nfs/callback_xdr.c +++ b/fs/nfs/callback_xdr.c @@ -1069,7 +1069,8 @@ static const struct svc_procedure nfs4_callback_procedures1[] = { } }; -static unsigned int nfs4_callback_count1[ARRAY_SIZE(nfs4_callback_procedures1)]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nfs4_callback_count1[ARRAY_SIZE(nfs4_callback_procedures1)]); const struct svc_version nfs4_callback_version1 = { .vs_vers = 1, .vs_nproc = ARRAY_SIZE(nfs4_callback_procedures1), @@ -1081,7 +1082,8 @@ const struct svc_version nfs4_callback_version1 = { .vs_need_cong_ctrl = true, }; -static unsigned int nfs4_callback_count4[ARRAY_SIZE(nfs4_callback_procedures1)]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nfs4_callback_count4[ARRAY_SIZE(nfs4_callback_procedures1)]); const struct svc_version nfs4_callback_version4 = { .vs_vers = 4, .vs_nproc = ARRAY_SIZE(nfs4_callback_procedures1), diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c index 1457f59f447a..dea55883e099 100644 --- a/fs/nfsd/nfs2acl.c +++ b/fs/nfsd/nfs2acl.c @@ -377,10 +377,11 @@ static const struct svc_procedure nfsd_acl_procedures2[5] = { }, }; -static unsigned int nfsd_acl_count2[ARRAY_SIZE(nfsd_acl_procedures2)]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nfsd_acl_count2[ARRAY_SIZE(nfsd_acl_procedures2)]); const struct svc_version nfsd_acl_version2 = { .vs_vers = 2, - .vs_nproc = 5, + .vs_nproc = ARRAY_SIZE(nfsd_acl_procedures2), .vs_proc = nfsd_acl_procedures2, .vs_count = nfsd_acl_count2, .vs_dispatch = nfsd_dispatch, diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c index 647108138e8a..bf1c52c1bdab 100644 --- a/fs/nfsd/nfs3acl.c +++ b/fs/nfsd/nfs3acl.c @@ -266,10 +266,11 @@ static const struct svc_procedure nfsd_acl_procedures3[3] = { }, }; -static unsigned int nfsd_acl_count3[ARRAY_SIZE(nfsd_acl_procedures3)]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nfsd_acl_count3[ARRAY_SIZE(nfsd_acl_procedures3)]); const struct svc_version nfsd_acl_version3 = { .vs_vers = 3, - .vs_nproc = 3, + .vs_nproc = ARRAY_SIZE(nfsd_acl_procedures3), .vs_proc = nfsd_acl_procedures3, .vs_count = nfsd_acl_count3, .vs_dispatch = nfsd_dispatch, diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index d01b29aba662..1c73921c02e0 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -1064,10 +1064,11 @@ static const struct svc_procedure nfsd_procedures3[22] = { }, }; -static unsigned int nfsd_count3[ARRAY_SIZE(nfsd_procedures3)]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nfsd_count3[ARRAY_SIZE(nfsd_procedures3)]); const struct svc_version nfsd_version3 = { .vs_vers = 3, - .vs_nproc = 22, + .vs_nproc = ARRAY_SIZE(nfsd_procedures3), .vs_proc = nfsd_procedures3, .vs_dispatch = nfsd_dispatch, .vs_count = nfsd_count3, diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index dbaf33398c82..9435136d7de6 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -3596,12 +3596,13 @@ static const struct svc_procedure nfsd_procedures4[2] = { }, }; -static unsigned int nfsd_count3[ARRAY_SIZE(nfsd_procedures4)]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nfsd_count4[ARRAY_SIZE(nfsd_procedures4)]); const struct svc_version nfsd_version4 = { .vs_vers = 4, - .vs_nproc = 2, + .vs_nproc = ARRAY_SIZE(nfsd_procedures4), .vs_proc = nfsd_procedures4, - .vs_count = nfsd_count3, + .vs_count = nfsd_count4, .vs_dispatch = nfsd_dispatch, .vs_xdrsize = NFS4_SVC_XDRSIZE, .vs_rpcb_optnl = true, diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c index 9744443c3965..9e6069e9c9f1 100644 --- a/fs/nfsd/nfsproc.c +++ b/fs/nfsd/nfsproc.c @@ -838,11 +838,11 @@ static const struct svc_procedure nfsd_procedures2[18] = { }, }; - -static unsigned int nfsd_count2[ARRAY_SIZE(nfsd_procedures2)]; +static DEFINE_PER_CPU_ALIGNED(unsigned long, + nfsd_count2[ARRAY_SIZE(nfsd_procedures2)]); const struct svc_version nfsd_version2 = { .vs_vers = 2, - .vs_nproc = 18, + .vs_nproc = ARRAY_SIZE(nfsd_procedures2), .vs_proc = nfsd_procedures2, .vs_count = nfsd_count2, .vs_dispatch = nfsd_dispatch, diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h index 70ce419e2709..84de6b7c9948 100644 --- a/include/linux/lockd/lockd.h +++ b/include/linux/lockd/lockd.h @@ -196,9 +196,9 @@ struct nlm_block { * Global variables */ extern const struct rpc_program nlm_program; -extern const struct svc_procedure nlmsvc_procedures[]; +extern const struct svc_procedure nlmsvc_procedures[24]; #ifdef CONFIG_LOCKD_V4 -extern const struct svc_procedure nlmsvc_procedures4[]; +extern const struct svc_procedure nlmsvc_procedures4[24]; #endif extern int nlmsvc_grace_period; extern unsigned long nlmsvc_timeout; diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h index 392d2d2620fa..1b078c9a2d60 100644 --- a/include/linux/sunrpc/svc.h +++ b/include/linux/sunrpc/svc.h @@ -377,7 +377,7 @@ struct svc_version { u32 vs_vers; /* version number */ u32 vs_nproc; /* number of procedures */ const struct svc_procedure *vs_proc; /* per-procedure info */ - unsigned int *vs_count; /* call counts */ + unsigned long __percpu *vs_count; /* call counts */ u32 vs_xdrsize; /* xdrsize needed for this version */ /* Don't register with rpcbind */ diff --git a/net/sunrpc/stats.c b/net/sunrpc/stats.c index 52908f9e6eab..65fc1297c6df 100644 --- a/net/sunrpc/stats.c +++ b/net/sunrpc/stats.c @@ -83,7 +83,8 @@ void svc_seq_show(struct seq_file *seq, const struct svc_stat *statp) { const struct svc_program *prog = statp->program; const struct svc_version *vers; - unsigned int i, j; + unsigned int i, j, k; + unsigned long count; seq_printf(seq, "net %u %u %u %u\n", @@ -104,8 +105,12 @@ void svc_seq_show(struct seq_file *seq, const struct svc_stat *statp) if (!vers) continue; seq_printf(seq, "proc%d %u", i, vers->vs_nproc); - for (j = 0; j < vers->vs_nproc; j++) - seq_printf(seq, " %u", vers->vs_count[j]); + for (j = 0; j < vers->vs_nproc; j++) { + count = 0; + for_each_possible_cpu(k) + count += per_cpu(vers->vs_count[j], k); + seq_printf(seq, " %lu", count); + } seq_putc(seq, '\n'); } } diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 7b208e063334..5dc298c30ea8 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -1208,7 +1208,7 @@ svc_generic_init_request(struct svc_rqst *rqstp, memset(rqstp->rq_resp, 0, procp->pc_ressize); /* Bump per-procedure stats counter */ - versp->vs_count[rqstp->rq_proc]++; + this_cpu_inc(versp->vs_count[rqstp->rq_proc]); ret->dispatch = versp->vs_dispatch; return rpc_success; -- cgit v1.2.3-70-g09d2 From 4df750c924f405fe773771add507117a80ae6203 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sun, 15 Jan 2023 12:21:39 -0500 Subject: NFSD: Replace /proc/fs/nfsd/supported_krb5_enctypes with a symlink Now that I've added a file under /proc/net/rpc that is managed by the SunRPC's Kerberos mechanism, replace NFSD's supported_krb5_enctypes file with a symlink to the new SunRPC proc file, which contains exactly the same content. Remarkably, commit b0b0c0a26e84 ("nfsd: add proc file listing kernel's gss_krb5 enctypes") added the nfsd_supported_krb5_enctypes file in 2011, but this file has never been documented in nfsd(7). Tested-by: Scott Mayhew Reviewed-by: Simo Sorce Signed-off-by: Chuck Lever --- fs/nfsd/nfsctl.c | 74 ++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 58 insertions(+), 16 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index f2a0d6ac88df..04474b8ccf0a 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -47,7 +46,6 @@ enum { NFSD_MaxBlkSize, NFSD_MaxConnections, NFSD_Filecache, - NFSD_SupportedEnctypes, /* * The below MUST come last. Otherwise we leave a hole in nfsd_files[] * with !CONFIG_NFSD_V4 and simple_fill_super() goes oops @@ -187,16 +185,6 @@ static int export_features_show(struct seq_file *m, void *v) DEFINE_SHOW_ATTRIBUTE(export_features); -#if defined(CONFIG_SUNRPC_GSS) || defined(CONFIG_SUNRPC_GSS_MODULE) -static int supported_enctypes_show(struct seq_file *m, void *v) -{ - seq_printf(m, KRB5_SUPPORTED_ENCTYPES); - return 0; -} - -DEFINE_SHOW_ATTRIBUTE(supported_enctypes); -#endif /* CONFIG_SUNRPC_GSS or CONFIG_SUNRPC_GSS_MODULE */ - static const struct file_operations pool_stats_operations = { .open = nfsd_pool_stats_open, .read = seq_read, @@ -1150,6 +1138,9 @@ static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode) inode->i_op = &simple_dir_inode_operations; inc_nlink(inode); break; + case S_IFLNK: + inode->i_op = &simple_symlink_inode_operations; + break; default: break; } @@ -1195,6 +1186,59 @@ out_err: goto out; } +#if IS_ENABLED(CONFIG_SUNRPC_GSS) +static int __nfsd_symlink(struct inode *dir, struct dentry *dentry, + umode_t mode, const char *content) +{ + struct inode *inode; + + inode = nfsd_get_inode(dir->i_sb, mode); + if (!inode) + return -ENOMEM; + + inode->i_link = (char *)content; + inode->i_size = strlen(content); + + d_add(dentry, inode); + inc_nlink(dir); + fsnotify_create(dir, dentry); + return 0; +} + +/* + * @content is assumed to be a NUL-terminated string that lives + * longer than the symlink itself. + */ +static void nfsd_symlink(struct dentry *parent, const char *name, + const char *content) +{ + struct inode *dir = parent->d_inode; + struct dentry *dentry; + int ret = -ENOMEM; + + inode_lock(dir); + dentry = d_alloc_name(parent, name); + if (!dentry) + goto out_err; + ret = __nfsd_symlink(d_inode(parent), dentry, S_IFLNK | 0777, content); + if (ret) + goto out_err; +out: + inode_unlock(dir); + return; +out_err: + dput(dentry); + dentry = ERR_PTR(ret); + goto out; +} +#else +static inline void nfsd_symlink(struct dentry *parent, const char *name, + const char *content) +{ +} + +#endif + static void clear_ncl(struct inode *inode) { struct nfsdfs_client *ncl = inode->i_private; @@ -1355,10 +1399,6 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc) [NFSD_MaxBlkSize] = {"max_block_size", &transaction_ops, S_IWUSR|S_IRUGO}, [NFSD_MaxConnections] = {"max_connections", &transaction_ops, S_IWUSR|S_IRUGO}, [NFSD_Filecache] = {"filecache", &nfsd_file_cache_stats_fops, S_IRUGO}, -#if defined(CONFIG_SUNRPC_GSS) || defined(CONFIG_SUNRPC_GSS_MODULE) - [NFSD_SupportedEnctypes] = {"supported_krb5_enctypes", - &supported_enctypes_fops, S_IRUGO}, -#endif /* CONFIG_SUNRPC_GSS or CONFIG_SUNRPC_GSS_MODULE */ #ifdef CONFIG_NFSD_V4 [NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR}, [NFSD_Gracetime] = {"nfsv4gracetime", &transaction_ops, S_IWUSR|S_IRUSR}, @@ -1371,6 +1411,8 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc) ret = simple_fill_super(sb, 0x6e667364, nfsd_files); if (ret) return ret; + nfsd_symlink(sb->s_root, "supported_krb5_enctypes", + "/proc/net/rpc/gss_krb5_enctypes"); dentry = nfsd_mkdir(sb->s_root, NULL, "clients"); if (IS_ERR(dentry)) return PTR_ERR(dentry); -- cgit v1.2.3-70-g09d2 From fcb530973b3c48099108e6e2e7433db9188f7eeb Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 18 Jan 2023 12:31:34 -0500 Subject: nfsd: don't take nfsd4_copy ref for OP_OFFLOAD_STATUS We're not doing any blocking operations for OP_OFFLOAD_STATUS, so taking and putting a reference is a waste of effort. Take the client lock, search for the copy and fetch the wr_bytes_written field and return. Also, make find_async_copy a static function. Signed-off-by: Jeff Layton Reviewed-by: Olga Kornievskaia Signed-off-by: Chuck Lever --- fs/nfsd/nfs4proc.c | 35 ++++++++++++++++++++++++----------- fs/nfsd/state.h | 2 -- 2 files changed, 24 insertions(+), 13 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 9435136d7de6..3b73e4d342bf 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1824,23 +1824,34 @@ out_err: goto out; } -struct nfsd4_copy * -find_async_copy(struct nfs4_client *clp, stateid_t *stateid) +static struct nfsd4_copy * +find_async_copy_locked(struct nfs4_client *clp, stateid_t *stateid) { struct nfsd4_copy *copy; - spin_lock(&clp->async_lock); + lockdep_assert_held(&clp->async_lock); + list_for_each_entry(copy, &clp->async_copies, copies) { if (memcmp(©->cp_stateid.cs_stid, stateid, NFS4_STATEID_SIZE)) continue; - refcount_inc(©->refcount); - spin_unlock(&clp->async_lock); return copy; } - spin_unlock(&clp->async_lock); return NULL; } +static struct nfsd4_copy * +find_async_copy(struct nfs4_client *clp, stateid_t *stateid) +{ + struct nfsd4_copy *copy; + + spin_lock(&clp->async_lock); + copy = find_async_copy_locked(clp, stateid); + if (copy) + refcount_inc(©->refcount); + spin_unlock(&clp->async_lock); + return copy; +} + static __be32 nfsd4_offload_cancel(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, @@ -1925,22 +1936,24 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, nfsd_file_put(nf); return status; } + static __be32 nfsd4_offload_status(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, union nfsd4_op_u *u) { struct nfsd4_offload_status *os = &u->offload_status; - __be32 status = 0; + __be32 status = nfs_ok; struct nfsd4_copy *copy; struct nfs4_client *clp = cstate->clp; - copy = find_async_copy(clp, &os->stateid); - if (copy) { + spin_lock(&clp->async_lock); + copy = find_async_copy_locked(clp, &os->stateid); + if (copy) os->count = copy->cp_res.wr_bytes_written; - nfs4_put_copy(copy); - } else + else status = nfserr_bad_stateid; + spin_unlock(&clp->async_lock); return status; } diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h index e94634d30591..d49d3060ed4f 100644 --- a/fs/nfsd/state.h +++ b/fs/nfsd/state.h @@ -705,8 +705,6 @@ extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(struct xdr_netobj name extern bool nfs4_has_reclaimed_state(struct xdr_netobj name, struct nfsd_net *nn); void put_nfs4_file(struct nfs4_file *fi); -extern struct nfsd4_copy * -find_async_copy(struct nfs4_client *clp, stateid_t *staetid); extern void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps); extern __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st, -- cgit v1.2.3-70-g09d2 From 45ba66cc2ca22a492fb16474d1af804388757a35 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 18 Jan 2023 12:31:35 -0500 Subject: nfsd: eliminate find_deleg_file_locked We really don't need an accessor function here. Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4state.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 7ed723994591..361049894a09 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -686,15 +686,6 @@ static struct nfsd_file *find_any_file_locked(struct nfs4_file *f) return NULL; } -static struct nfsd_file *find_deleg_file_locked(struct nfs4_file *f) -{ - lockdep_assert_held(&f->fi_lock); - - if (f->fi_deleg_file) - return f->fi_deleg_file; - return NULL; -} - static atomic_long_t num_delegations; unsigned long max_delegations; @@ -2703,7 +2694,7 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st) ds = delegstateid(st); nf = st->sc_file; spin_lock(&nf->fi_lock); - file = find_deleg_file_locked(nf); + file = nf->fi_deleg_file; if (!file) goto out; -- cgit v1.2.3-70-g09d2 From ee97e7301582e3100d396e1cf45c7f5a74c7be97 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 18 Jan 2023 12:31:38 -0500 Subject: nfsd: add some kerneldoc comments for stateid preprocessing functions Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4state.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 361049894a09..7d8bd0bb0a0e 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -6539,8 +6539,19 @@ void nfs4_put_cpntf_state(struct nfsd_net *nn, struct nfs4_cpntf_state *cps) spin_unlock(&nn->s2s_cp_lock); } -/* - * Checks for stateid operations +/** + * nfs4_preprocess_stateid_op - find and prep stateid for an operation + * @rqstp: incoming request from client + * @cstate: current compound state + * @fhp: filehandle associated with requested stateid + * @stateid: stateid (provided by client) + * @flags: flags describing type of operation to be done + * @nfp: optional nfsd_file return pointer (may be NULL) + * @cstid: optional returned nfs4_stid pointer (may be NULL) + * + * Given info from the client, look up a nfs4_stid for the operation. On + * success, it returns a reference to the nfs4_stid and/or the nfsd_file + * associated with it. */ __be32 nfs4_preprocess_stateid_op(struct svc_rqst *rqstp, @@ -6729,8 +6740,18 @@ static __be32 nfs4_seqid_op_checks(struct nfsd4_compound_state *cstate, stateid_ return status; } -/* - * Checks for sequence id mutating operations. +/** + * nfs4_preprocess_seqid_op - find and prep an ol_stateid for a seqid-morphing op + * @cstate: compund state + * @seqid: seqid (provided by client) + * @stateid: stateid (provided by client) + * @typemask: mask of allowable types for this operation + * @stpp: return pointer for the stateid found + * @nn: net namespace for request + * + * Given a stateid+seqid from a client, look up an nfs4_ol_stateid and + * return it in @stpp. On a nfs_ok return, the returned stateid will + * have its st_mutex locked. */ static __be32 nfs4_preprocess_seqid_op(struct nfsd4_compound_state *cstate, u32 seqid, -- cgit v1.2.3-70-g09d2 From edd2f5526ea87cf6fc66a08bc8a1fbb6022a502f Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 18 Jan 2023 12:31:39 -0500 Subject: nfsd: eliminate __nfs4_get_fd This is wrapper is pointless, and just obscures what's going on. Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4state.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 7d8bd0bb0a0e..2143d0b50d63 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -599,12 +599,6 @@ put_nfs4_file(struct nfs4_file *fi) } } -static struct nfsd_file * -__nfs4_get_fd(struct nfs4_file *f, int oflag) -{ - return nfsd_file_get(f->fi_fds[oflag]); -} - static struct nfsd_file * find_writeable_file_locked(struct nfs4_file *f) { @@ -612,9 +606,9 @@ find_writeable_file_locked(struct nfs4_file *f) lockdep_assert_held(&f->fi_lock); - ret = __nfs4_get_fd(f, O_WRONLY); + ret = nfsd_file_get(f->fi_fds[O_WRONLY]); if (!ret) - ret = __nfs4_get_fd(f, O_RDWR); + ret = nfsd_file_get(f->fi_fds[O_RDWR]); return ret; } @@ -637,9 +631,9 @@ find_readable_file_locked(struct nfs4_file *f) lockdep_assert_held(&f->fi_lock); - ret = __nfs4_get_fd(f, O_RDONLY); + ret = nfsd_file_get(f->fi_fds[O_RDONLY]); if (!ret) - ret = __nfs4_get_fd(f, O_RDWR); + ret = nfsd_file_get(f->fi_fds[O_RDWR]); return ret; } @@ -663,11 +657,11 @@ find_any_file(struct nfs4_file *f) if (!f) return NULL; spin_lock(&f->fi_lock); - ret = __nfs4_get_fd(f, O_RDWR); + ret = nfsd_file_get(f->fi_fds[O_RDWR]); if (!ret) { - ret = __nfs4_get_fd(f, O_WRONLY); + ret = nfsd_file_get(f->fi_fds[O_WRONLY]); if (!ret) - ret = __nfs4_get_fd(f, O_RDONLY); + ret = nfsd_file_get(f->fi_fds[O_RDONLY]); } spin_unlock(&f->fi_lock); return ret; -- cgit v1.2.3-70-g09d2 From 1f0001d43d0c0ac2a19a34a914f6595ad97cbc1d Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 17 Jan 2023 14:38:30 -0500 Subject: nfsd: zero out pointers after putting nfsd_files on COPY setup error At first, I thought this might be a source of nfsd_file overputs, but the current callers seem to avoid an extra put when nfsd4_verify_copy returns an error. Still, it's "bad form" to leave the pointers filled out when we don't have a reference to them anymore, and that might lead to bugs later. Zero them out as a defensive coding measure. Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4proc.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs') diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 3b73e4d342bf..95234c8060d5 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1214,8 +1214,10 @@ out: return status; out_put_dst: nfsd_file_put(*dst); + *dst = NULL; out_put_src: nfsd_file_put(*src); + *src = NULL; goto out; } -- cgit v1.2.3-70-g09d2 From 6ba434cb1a8d403ea9aad1b667c3ea3ad8b3191f Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 17 Jan 2023 14:38:31 -0500 Subject: nfsd: clean up potential nfsd_file refcount leaks in COPY codepath There are two different flavors of the nfsd4_copy struct. One is embedded in the compound and is used directly in synchronous copies. The other is dynamically allocated, refcounted and tracked in the client struture. For the embedded one, the cleanup just involves releasing any nfsd_files held on its behalf. For the async one, the cleanup is a bit more involved, and we need to dequeue it from lists, unhash it, etc. There is at least one potential refcount leak in this code now. If the kthread_create call fails, then both the src and dst nfsd_files in the original nfsd4_copy object are leaked. The cleanup in this codepath is also sort of weird. In the async copy case, we'll have up to four nfsd_file references (src and dst for both flavors of copy structure). They are both put at the end of nfsd4_do_async_copy, even though the ones held on behalf of the embedded one outlive that structure. Change it so that we always clean up the nfsd_file refs held by the embedded copy structure before nfsd4_copy returns. Rework cleanup_async_copy to handle both inter and intra copies. Eliminate nfsd4_cleanup_intra_ssc since it now becomes a no-op. Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4proc.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 95234c8060d5..c9057462b973 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1512,7 +1512,6 @@ nfsd4_cleanup_inter_ssc(struct nfsd4_ssc_umount_item *nsui, struct file *filp, long timeout = msecs_to_jiffies(nfsd4_ssc_umount_timeout); nfs42_ssc_close(filp); - nfsd_file_put(dst); fput(filp); spin_lock(&nn->nfsd_ssc_lock); @@ -1562,13 +1561,6 @@ nfsd4_setup_intra_ssc(struct svc_rqst *rqstp, ©->nf_dst); } -static void -nfsd4_cleanup_intra_ssc(struct nfsd_file *src, struct nfsd_file *dst) -{ - nfsd_file_put(src); - nfsd_file_put(dst); -} - static void nfsd4_cb_offload_release(struct nfsd4_callback *cb) { struct nfsd4_cb_offload *cbo = @@ -1683,12 +1675,18 @@ static void dup_copy_fields(struct nfsd4_copy *src, struct nfsd4_copy *dst) dst->ss_nsui = src->ss_nsui; } +static void release_copy_files(struct nfsd4_copy *copy) +{ + if (copy->nf_src) + nfsd_file_put(copy->nf_src); + if (copy->nf_dst) + nfsd_file_put(copy->nf_dst); +} + static void cleanup_async_copy(struct nfsd4_copy *copy) { nfs4_free_copy_state(copy); - nfsd_file_put(copy->nf_dst); - if (!nfsd4_ssc_is_inter(copy)) - nfsd_file_put(copy->nf_src); + release_copy_files(copy); spin_lock(©->cp_clp->async_lock); list_del(©->copies); spin_unlock(©->cp_clp->async_lock); @@ -1748,7 +1746,6 @@ static int nfsd4_do_async_copy(void *data) } else { nfserr = nfsd4_do_copy(copy, copy->nf_src->nf_file, copy->nf_dst->nf_file, false); - nfsd4_cleanup_intra_ssc(copy->nf_src, copy->nf_dst); } do_callback: @@ -1811,9 +1808,9 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, } else { status = nfsd4_do_copy(copy, copy->nf_src->nf_file, copy->nf_dst->nf_file, true); - nfsd4_cleanup_intra_ssc(copy->nf_src, copy->nf_dst); } out: + release_copy_files(copy); return status; out_err: if (async_copy) -- cgit v1.2.3-70-g09d2 From 34e8f9ec4c9ac235f917747b23a200a5e0ec857b Mon Sep 17 00:00:00 2001 From: Dai Ngo Date: Mon, 23 Jan 2023 21:34:13 -0800 Subject: NFSD: fix leaked reference count of nfsd4_ssc_umount_item The reference count of nfsd4_ssc_umount_item is not decremented on error conditions. This prevents the laundromat from unmounting the vfsmount of the source file. This patch decrements the reference count of nfsd4_ssc_umount_item on error. Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.") Signed-off-by: Dai Ngo Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4proc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index c9057462b973..57f791899de3 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1813,13 +1813,17 @@ out: release_copy_files(copy); return status; out_err: + if (nfsd4_ssc_is_inter(copy)) { + /* + * Source's vfsmount of inter-copy will be unmounted + * by the laundromat. Use copy instead of async_copy + * since async_copy->ss_nsui might not be set yet. + */ + refcount_dec(©->ss_nsui->nsui_refcnt); + } if (async_copy) cleanup_async_copy(async_copy); status = nfserrno(-ENOMEM); - /* - * source's vfsmount of inter-copy will be unmounted - * by the laundromat - */ goto out; } -- cgit v1.2.3-70-g09d2 From 4dbca1c3c6dd0f21b1b6c5898b7579d465d30468 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 24 Jan 2023 11:45:38 -0500 Subject: nfsd: remove fs/nfsd/fault_inject.c This file is no longer built at all. Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/fault_inject.c | 142 ------------------------------------------------- 1 file changed, 142 deletions(-) delete mode 100644 fs/nfsd/fault_inject.c (limited to 'fs') diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c deleted file mode 100644 index 76bee0a0d308..000000000000 --- a/fs/nfsd/fault_inject.c +++ /dev/null @@ -1,142 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * Copyright (c) 2011 Bryan Schumaker - * - * Uses debugfs to create fault injection points for client testing - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include "state.h" -#include "netns.h" - -struct nfsd_fault_inject_op { - char *file; - u64 (*get)(void); - u64 (*set_val)(u64); - u64 (*set_clnt)(struct sockaddr_storage *, size_t); -}; - -static struct dentry *debug_dir; - -static ssize_t fault_inject_read(struct file *file, char __user *buf, - size_t len, loff_t *ppos) -{ - static u64 val; - char read_buf[25]; - size_t size; - loff_t pos = *ppos; - struct nfsd_fault_inject_op *op = file_inode(file)->i_private; - - if (!pos) - val = op->get(); - size = scnprintf(read_buf, sizeof(read_buf), "%llu\n", val); - - return simple_read_from_buffer(buf, len, ppos, read_buf, size); -} - -static ssize_t fault_inject_write(struct file *file, const char __user *buf, - size_t len, loff_t *ppos) -{ - char write_buf[INET6_ADDRSTRLEN]; - size_t size = min(sizeof(write_buf) - 1, len); - struct net *net = current->nsproxy->net_ns; - struct sockaddr_storage sa; - struct nfsd_fault_inject_op *op = file_inode(file)->i_private; - u64 val; - char *nl; - - if (copy_from_user(write_buf, buf, size)) - return -EFAULT; - write_buf[size] = '\0'; - - /* Deal with any embedded newlines in the string */ - nl = strchr(write_buf, '\n'); - if (nl) { - size = nl - write_buf; - *nl = '\0'; - } - - size = rpc_pton(net, write_buf, size, (struct sockaddr *)&sa, sizeof(sa)); - if (size > 0) { - val = op->set_clnt(&sa, size); - if (val) - pr_info("NFSD [%s]: Client %s had %llu state object(s)\n", - op->file, write_buf, val); - } else { - val = simple_strtoll(write_buf, NULL, 0); - if (val == 0) - pr_info("NFSD Fault Injection: %s (all)", op->file); - else - pr_info("NFSD Fault Injection: %s (n = %llu)", - op->file, val); - val = op->set_val(val); - pr_info("NFSD: %s: found %llu", op->file, val); - } - return len; /* on success, claim we got the whole input */ -} - -static const struct file_operations fops_nfsd = { - .owner = THIS_MODULE, - .read = fault_inject_read, - .write = fault_inject_write, -}; - -void nfsd_fault_inject_cleanup(void) -{ - debugfs_remove_recursive(debug_dir); -} - -static struct nfsd_fault_inject_op inject_ops[] = { - { - .file = "forget_clients", - .get = nfsd_inject_print_clients, - .set_val = nfsd_inject_forget_clients, - .set_clnt = nfsd_inject_forget_client, - }, - { - .file = "forget_locks", - .get = nfsd_inject_print_locks, - .set_val = nfsd_inject_forget_locks, - .set_clnt = nfsd_inject_forget_client_locks, - }, - { - .file = "forget_openowners", - .get = nfsd_inject_print_openowners, - .set_val = nfsd_inject_forget_openowners, - .set_clnt = nfsd_inject_forget_client_openowners, - }, - { - .file = "forget_delegations", - .get = nfsd_inject_print_delegations, - .set_val = nfsd_inject_forget_delegations, - .set_clnt = nfsd_inject_forget_client_delegations, - }, - { - .file = "recall_delegations", - .get = nfsd_inject_print_delegations, - .set_val = nfsd_inject_recall_delegations, - .set_clnt = nfsd_inject_recall_client_delegations, - }, -}; - -void nfsd_fault_inject_init(void) -{ - unsigned int i; - struct nfsd_fault_inject_op *op; - umode_t mode = S_IFREG | S_IRUSR | S_IWUSR; - - debug_dir = debugfs_create_dir("nfsd", NULL); - - for (i = 0; i < ARRAY_SIZE(inject_ops); i++) { - op = &inject_ops[i]; - debugfs_create_file(op->file, mode, debug_dir, op, &fops_nfsd); - } -} -- cgit v1.2.3-70-g09d2 From 826b67e6376c2a788e3a62c4860dcd79500a27d5 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 27 Jan 2023 07:09:33 -0500 Subject: nfsd: don't hand out delegation on setuid files being opened for write We had a bug report that xfstest generic/355 was failing on NFSv4.0. This test sets various combinations of setuid/setgid modes and tests whether DIO writes will cause them to be stripped. What I found was that the server did properly strip those bits, but the client didn't notice because it held a delegation that was not recalled. The recall didn't occur because the client itself was the one generating the activity and we avoid recalls in that case. Clearing setuid bits is an "implicit" activity. The client didn't specifically request that we do that, so we need the server to issue a CB_RECALL, or avoid the situation entirely by not issuing a delegation. The easiest fix here is to simply not give out a delegation if the file is being opened for write, and the mode has the setuid and/or setgid bit set. Note that there is a potential race between the mode and lease being set, so we test for this condition both before and after setting the lease. This patch fixes generic/355, generic/683 and generic/684 for me. (Note that 355 fails only on v4.0, and 683 and 684 require NFSv4.2 to run and fail). Reported-by: Boyang Xue Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4state.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'fs') diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 2143d0b50d63..5955a8b23a3d 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -5421,6 +5421,23 @@ nfsd4_verify_deleg_dentry(struct nfsd4_open *open, struct nfs4_file *fp, return 0; } +/* + * We avoid breaking delegations held by a client due to its own activity, but + * clearing setuid/setgid bits on a write is an implicit activity and the client + * may not notice and continue using the old mode. Avoid giving out a delegation + * on setuid/setgid files when the client is requesting an open for write. + */ +static int +nfsd4_verify_setuid_write(struct nfsd4_open *open, struct nfsd_file *nf) +{ + struct inode *inode = file_inode(nf->nf_file); + + if ((open->op_share_access & NFS4_SHARE_ACCESS_WRITE) && + (inode->i_mode & (S_ISUID|S_ISGID))) + return -EAGAIN; + return 0; +} + static struct nfs4_delegation * nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, struct svc_fh *parent) @@ -5454,6 +5471,8 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, spin_lock(&fp->fi_lock); if (nfs4_delegation_exists(clp, fp)) status = -EAGAIN; + else if (nfsd4_verify_setuid_write(open, nf)) + status = -EAGAIN; else if (!fp->fi_deleg_file) { fp->fi_deleg_file = nf; /* increment early to prevent fi_deleg_file from being @@ -5494,6 +5513,14 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, if (status) goto out_unlock; + /* + * Now that the deleg is set, check again to ensure that nothing + * raced in and changed the mode while we weren't lookng. + */ + status = nfsd4_verify_setuid_write(open, fp->fi_deleg_file); + if (status) + goto out_unlock; + spin_lock(&state_lock); spin_lock(&fp->fi_lock); if (fp->fi_had_conflict) -- cgit v1.2.3-70-g09d2 From fb610c4dbc996415d57d7090957ecddd4fd64fb6 Mon Sep 17 00:00:00 2001 From: Benjamin Coddington Date: Fri, 27 Jan 2023 11:18:56 -0500 Subject: nfsd: fix race to check ls_layouts Its possible for __break_lease to find the layout's lease before we've added the layout to the owner's ls_layouts list. In that case, setting ls_recalled = true without actually recalling the layout will cause the server to never send a recall callback. Move the check for ls_layouts before setting ls_recalled. Fixes: c5c707f96fc9 ("nfsd: implement pNFS layout recalls") Signed-off-by: Benjamin Coddington Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4layouts.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c index 3564d1c6f610..e8a80052cb1b 100644 --- a/fs/nfsd/nfs4layouts.c +++ b/fs/nfsd/nfs4layouts.c @@ -323,11 +323,11 @@ nfsd4_recall_file_layout(struct nfs4_layout_stateid *ls) if (ls->ls_recalled) goto out_unlock; - ls->ls_recalled = true; - atomic_inc(&ls->ls_stid.sc_file->fi_lo_recalls); if (list_empty(&ls->ls_layouts)) goto out_unlock; + ls->ls_recalled = true; + atomic_inc(&ls->ls_stid.sc_file->fi_lo_recalls); trace_nfsd_layout_recall(&ls->ls_stid.sc_stateid); refcount_inc(&ls->ls_stid.sc_count); -- cgit v1.2.3-70-g09d2 From 81e722978ad21072470b73d8f6a50ad62c7d5b7d Mon Sep 17 00:00:00 2001 From: Dai Ngo Date: Tue, 31 Jan 2023 11:12:29 -0800 Subject: NFSD: fix problems with cleanup on errors in nfsd4_copy When nfsd4_copy fails to allocate memory for async_copy->cp_src, or nfs4_init_copy_state fails, it calls cleanup_async_copy to do the cleanup for the async_copy which causes page fault since async_copy is not yet initialized. This patche rearranges the order of initializing the fields in async_copy and adds checks in cleanup_async_copy to skip un-initialized fields. Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy") Fixes: 87689df69491 ("NFSD: Shrink size of struct nfsd4_copy") Signed-off-by: Dai Ngo Reviewed-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfs4proc.c | 12 ++++++++---- fs/nfsd/nfs4state.c | 5 +++-- 2 files changed, 11 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 57f791899de3..5ae670807449 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1687,9 +1687,12 @@ static void cleanup_async_copy(struct nfsd4_copy *copy) { nfs4_free_copy_state(copy); release_copy_files(copy); - spin_lock(©->cp_clp->async_lock); - list_del(©->copies); - spin_unlock(©->cp_clp->async_lock); + if (copy->cp_clp) { + spin_lock(©->cp_clp->async_lock); + if (!list_empty(©->copies)) + list_del_init(©->copies); + spin_unlock(©->cp_clp->async_lock); + } nfs4_put_copy(copy); } @@ -1786,12 +1789,13 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, async_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL); if (!async_copy) goto out_err; + INIT_LIST_HEAD(&async_copy->copies); + refcount_set(&async_copy->refcount, 1); async_copy->cp_src = kmalloc(sizeof(*async_copy->cp_src), GFP_KERNEL); if (!async_copy->cp_src) goto out_err; if (!nfs4_init_copy_state(nn, copy)) goto out_err; - refcount_set(&async_copy->refcount, 1); memcpy(©->cp_res.cb_stateid, ©->cp_stateid.cs_stid, sizeof(copy->cp_res.cb_stateid)); dup_copy_fields(copy, async_copy); diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 5955a8b23a3d..b9dc47c051cc 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -975,7 +975,6 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid, stid->cs_stid.si_opaque.so_clid.cl_boot = (u32)nn->boot_time; stid->cs_stid.si_opaque.so_clid.cl_id = nn->s2s_cp_cl_id; - stid->cs_type = cs_type; idr_preload(GFP_KERNEL); spin_lock(&nn->s2s_cp_lock); @@ -986,6 +985,7 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid, idr_preload_end(); if (new_id < 0) return 0; + stid->cs_type = cs_type; return 1; } @@ -1019,7 +1019,8 @@ void nfs4_free_copy_state(struct nfsd4_copy *copy) { struct nfsd_net *nn; - WARN_ON_ONCE(copy->cp_stateid.cs_type != NFS4_COPY_STID); + if (copy->cp_stateid.cs_type != NFS4_COPY_STID) + return; nn = net_generic(copy->cp_clp->net, nfsd_net_id); spin_lock(&nn->s2s_cp_lock); idr_remove(&nn->s2s_cp_stateids, -- cgit v1.2.3-70-g09d2 From dcd779dc46540e174a6ac8d52fbed23593407317 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 3 Feb 2023 13:18:34 -0500 Subject: nfsd: fix courtesy client with deny mode handling in nfs4_upgrade_open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nested if statements here make no sense, as you can never reach "else" branch in the nested statement. Fix the error handling for when there is a courtesy client that holds a conflicting deny mode. Fixes: 3d6942715180 ("NFSD: add support for share reservation conflict to courteous server") Reported-by: 張智諺 Signed-off-by: Jeff Layton Reviewed-by: Dai Ngo Signed-off-by: Chuck Lever --- fs/nfsd/nfs4state.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index b9dc47c051cc..a202be19f26f 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -5282,16 +5282,17 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, /* test and set deny mode */ spin_lock(&fp->fi_lock); status = nfs4_file_check_deny(fp, open->op_share_deny); - if (status == nfs_ok) { - if (status != nfserr_share_denied) { - set_deny(open->op_share_deny, stp); - fp->fi_share_deny |= - (open->op_share_deny & NFS4_SHARE_DENY_BOTH); - } else { - if (nfs4_resolve_deny_conflicts_locked(fp, false, - stp, open->op_share_deny, false)) - status = nfserr_jukebox; - } + switch (status) { + case nfs_ok: + set_deny(open->op_share_deny, stp); + fp->fi_share_deny |= + (open->op_share_deny & NFS4_SHARE_DENY_BOTH); + break; + case nfserr_share_denied: + if (nfs4_resolve_deny_conflicts_locked(fp, false, + stp, open->op_share_deny, false)) + status = nfserr_jukebox; + break; } spin_unlock(&fp->fi_lock); -- cgit v1.2.3-70-g09d2 From 4c475eee02375ade6e864f1db16976ba0d96a0a2 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 7 Feb 2023 12:02:46 -0500 Subject: nfsd: don't fsync nfsd_files on last close Most of the time, NFSv4 clients issue a COMMIT before the final CLOSE of an open stateid, so with NFSv4, the fsync in the nfsd_file_free path is usually a no-op and doesn't block. We have a customer running knfsd over very slow storage (XFS over Ceph RBD). They were using the "async" export option because performance was more important than data integrity for this application. That export option turns NFSv4 COMMIT calls into no-ops. Due to the fsync in this codepath however, their final CLOSE calls would still stall (since a CLOSE effectively became a COMMIT). I think this fsync is not strictly necessary. We only use that result to reset the write verifier. Instead of fsync'ing all of the data when we free an nfsd_file, we can just check for writeback errors when one is acquired and when it is freed. If the client never comes back, then it'll never see the error anyway and there is no point in resetting it. If an error occurs after the nfsd_file is removed from the cache but before the inode is evicted, then it will reset the write verifier on the next nfsd_file_acquire, (since there will be an unseen error). The only exception here is if something else opens and fsyncs the file during that window. Given that local applications work with this limitation today, I don't see that as an issue. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2166658 Fixes: ac3a2585f018 ("nfsd: rework refcounting in filecache") Reported-and-tested-by: Pierguido Lambri Signed-off-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/filecache.c | 44 ++++++++++++-------------------------------- fs/nfsd/trace.h | 31 ------------------------------- 2 files changed, 12 insertions(+), 63 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c index ecc32105c3ab..6e8712bd7c99 100644 --- a/fs/nfsd/filecache.c +++ b/fs/nfsd/filecache.c @@ -331,37 +331,27 @@ nfsd_file_alloc(struct nfsd_file_lookup_key *key, unsigned int may) return nf; } +/** + * nfsd_file_check_write_error - check for writeback errors on a file + * @nf: nfsd_file to check for writeback errors + * + * Check whether a nfsd_file has an unseen error. Reset the write + * verifier if so. + */ static void -nfsd_file_fsync(struct nfsd_file *nf) -{ - struct file *file = nf->nf_file; - int ret; - - if (!file || !(file->f_mode & FMODE_WRITE)) - return; - ret = vfs_fsync(file, 1); - trace_nfsd_file_fsync(nf, ret); - if (ret) - nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id)); -} - -static int nfsd_file_check_write_error(struct nfsd_file *nf) { struct file *file = nf->nf_file; - if (!file || !(file->f_mode & FMODE_WRITE)) - return 0; - return filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err)); + if ((file->f_mode & FMODE_WRITE) && + filemap_check_wb_err(file->f_mapping, READ_ONCE(file->f_wb_err))) + nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id)); } static void nfsd_file_hash_remove(struct nfsd_file *nf) { trace_nfsd_file_unhash(nf); - - if (nfsd_file_check_write_error(nf)) - nfsd_reset_write_verifier(net_generic(nf->nf_net, nfsd_net_id)); rhashtable_remove_fast(&nfsd_file_rhash_tbl, &nf->nf_rhash, nfsd_file_rhash_params); } @@ -387,23 +377,12 @@ nfsd_file_free(struct nfsd_file *nf) this_cpu_add(nfsd_file_total_age, age); nfsd_file_unhash(nf); - - /* - * We call fsync here in order to catch writeback errors. It's not - * strictly required by the protocol, but an nfsd_file could get - * evicted from the cache before a COMMIT comes in. If another - * task were to open that file in the interim and scrape the error, - * then the client may never see it. By calling fsync here, we ensure - * that writeback happens before the entry is freed, and that any - * errors reported result in the write verifier changing. - */ - nfsd_file_fsync(nf); - if (nf->nf_mark) nfsd_file_mark_put(nf->nf_mark); if (nf->nf_file) { get_file(nf->nf_file); filp_close(nf->nf_file, NULL); + nfsd_file_check_write_error(nf); fput(nf->nf_file); } @@ -1158,6 +1137,7 @@ wait_for_construction: out: if (status == nfs_ok) { this_cpu_inc(nfsd_file_acquisitions); + nfsd_file_check_write_error(nf); *pnf = nf; } else { if (refcount_dec_and_test(&nf->nf_ref)) diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h index 8f9c82d9e075..4183819ea082 100644 --- a/fs/nfsd/trace.h +++ b/fs/nfsd/trace.h @@ -1202,37 +1202,6 @@ TRACE_EVENT(nfsd_file_close, ) ); -TRACE_EVENT(nfsd_file_fsync, - TP_PROTO( - const struct nfsd_file *nf, - int ret - ), - TP_ARGS(nf, ret), - TP_STRUCT__entry( - __field(void *, nf_inode) - __field(int, nf_ref) - __field(int, ret) - __field(unsigned long, nf_flags) - __field(unsigned char, nf_may) - __field(struct file *, nf_file) - ), - TP_fast_assign( - __entry->nf_inode = nf->nf_inode; - __entry->nf_ref = refcount_read(&nf->nf_ref); - __entry->ret = ret; - __entry->nf_flags = nf->nf_flags; - __entry->nf_may = nf->nf_may; - __entry->nf_file = nf->nf_file; - ), - TP_printk("inode=%p ref=%d flags=%s may=%s nf_file=%p ret=%d", - __entry->nf_inode, - __entry->nf_ref, - show_nf_flags(__entry->nf_flags), - show_nfsd_may_flags(__entry->nf_may), - __entry->nf_file, __entry->ret - ) -); - #include "cache.h" TRACE_DEFINE_ENUM(RC_DROPIT); -- cgit v1.2.3-70-g09d2 From 90d2175572470ba7f55da8447c72ddd4942923c4 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 14 Feb 2023 10:07:59 -0500 Subject: NFSD: copy the whole verifier in nfsd_copy_write_verifier Currently, we're only memcpy'ing the first __be32. Ensure we copy into both words. Fixes: 91d2e9b56cf5 ("NFSD: Clean up the nfsd_net::nfssvc_boot field") Reported-by: Jeff Layton Signed-off-by: Chuck Lever --- fs/nfsd/nfssvc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index fe5e4f73bb98..9c7b1ef5be40 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -363,7 +363,7 @@ void nfsd_copy_write_verifier(__be32 verf[2], struct nfsd_net *nn) do { read_seqbegin_or_lock(&nn->writeverf_lock, &seq); - memcpy(verf, nn->writeverf, sizeof(*verf)); + memcpy(verf, nn->writeverf, sizeof(nn->writeverf)); } while (need_seqretry(&nn->writeverf_lock, seq)); done_seqretry(&nn->writeverf_lock, seq); } -- cgit v1.2.3-70-g09d2 From 4b471a8b847b82a3035709dcf87661915c340c8a Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Tue, 14 Feb 2023 10:19:30 -0500 Subject: NFSD: Clean up nfsd_symlink() The pointer dentry is assigned a value that is never read, the assignment is redundant and can be removed. Cleans up clang-scan warning: fs/nfsd/nfsctl.c:1231:2: warning: Value stored to 'dentry' is never read [deadcode.DeadStores] dentry = ERR_PTR(ret); No need to initialize "int ret = -ENOMEM;" either. These are vestiges of nfsd_mkdir(), from whence I copied nfsd_symlink(). Reported-by: Colin Ian King Reported-by: Dan Carpenter Signed-off-by: Chuck Lever --- fs/nfsd/nfsctl.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'fs') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 04474b8ccf0a..7b8f17ee5224 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1214,22 +1214,17 @@ static void nfsd_symlink(struct dentry *parent, const char *name, { struct inode *dir = parent->d_inode; struct dentry *dentry; - int ret = -ENOMEM; + int ret; inode_lock(dir); dentry = d_alloc_name(parent, name); if (!dentry) - goto out_err; + goto out; ret = __nfsd_symlink(d_inode(parent), dentry, S_IFLNK | 0777, content); if (ret) - goto out_err; + dput(dentry); out: inode_unlock(dir); - return; -out_err: - dput(dentry); - dentry = ERR_PTR(ret); - goto out; } #else static inline void nfsd_symlink(struct dentry *parent, const char *name, -- cgit v1.2.3-70-g09d2