Nit: this could use a better subject line.
On Tue, Oct 24, 2017 at 01:47:47PM -0400, Olga Kornievskaia wrote:
...
> + if (!copy->cp_synchronous) {
> + status = nfsd4_init_copy_res(copy, 0);
> + async_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL);
> + if (!async_copy) {
> + status = nfserrno(-ENOMEM);
> + goto out;
> + }
> + dup_copy_fields(copy, async_copy);
> + memcpy(©->cp_res.cb_stateid, ©->cp_dst_stateid,
> + sizeof(copy->cp_dst_stateid));
> + spin_lock(&async_copy->cp_clp->async_lock);
> + list_add(&async_copy->copies,
> + &async_copy->cp_clp->async_copies);
> + spin_unlock(&async_copy->cp_clp->async_lock);
At this point other threads could in theory look up this async_copy, but
its copy_task field is not yet initialized. I don't *think* that's a
problem for nfsd4_shutdown_copy, because I don't think the server could
be processing rpc's for this client any more at that point. But I think
a malicious client might be able to trigger a NULL dereference in
nfsd4_offload_cancel.
Is there any reason not to assign copy_task before adding it to this
list?
--b.
> + async_copy->copy_task = kthread_create(nfsd4_do_async_copy,
> + async_copy, "%s", "copy thread");
> + if (IS_ERR(async_copy->copy_task)) {
> + status = PTR_ERR(async_copy->copy_task);
> + goto out_err_dec;
> + }
> + wake_up_process(async_copy->copy_task);
> + } else {
> + status = nfsd4_do_copy(copy, 1);
> }
> -
> - fput(src);
> - fput(dst);
> out:
> return status;
> +out_err_dec:
> + cleanup_async_copy(async_copy);
> + goto out;
> }
>
> static __be32
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 0c04f81..d7767a1 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1774,6 +1774,8 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
> #ifdef CONFIG_NFSD_PNFS
> INIT_LIST_HEAD(&clp->cl_lo_states);
> #endif
> + INIT_LIST_HEAD(&clp->async_copies);
> + spin_lock_init(&clp->async_lock);
> spin_lock_init(&clp->cl_lock);
> rpc_init_wait_queue(&clp->cl_cb_waitq, "Backchannel slot table");
> return clp;
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index f8b0210..9189062 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -352,6 +352,8 @@ struct nfs4_client {
> struct rpc_wait_queue cl_cb_waitq; /* backchannel callers may */
> /* wait here for slots */
> struct net *net;
> + struct list_head async_copies; /* list of async copies */
> + spinlock_t async_lock; /* lock for async copies */
> };
>
> /* struct nfs4_client_reset
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 9b0c099..0a19954 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -529,6 +529,15 @@ struct nfsd4_copy {
> struct nfsd4_callback cp_cb;
> __be32 nfserr;
> struct knfsd_fh fh;
> +
> + struct nfs4_client *cp_clp;
> +
> + struct file *fh_src;
> + struct file *fh_dst;
> + struct net *net;
> +
> + struct list_head copies;
> + struct task_struct *copy_task;
> };
>
> struct nfsd4_seek {
> --
> 1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
