Hi Scott,
Thanks. This mostly looks good, but I do have 1 comment below.
On Mon, 2021-11-01 at 16:06 -0400, Scott Mayhew wrote:
> During umount, the session slot tables are freed. If there are
> outstanding FREE_STATEID tasks, a use-after-free and slab corruption
> can
> occur when rpc_exit_task calls rpc_call_done -> nfs41_sequence_done -
> >
> nfs4_sequence_process/nfs41_sequence_free_slot.
>
> Prevent that from happening by taking a reference on the nfs_client
> in
> nfs41_free_stateid and putting it in nfs41_free_stateid_release.
>
> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index e1214bb6b7ee..76e6786b797e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -10145,18 +10145,24 @@ static void
> nfs41_free_stateid_prepare(struct rpc_task *task, void *calldata)
> static void nfs41_free_stateid_done(struct rpc_task *task, void
> *calldata)
> {
> struct nfs_free_stateid_data *data = calldata;
> + struct nfs_client *clp = data->server->nfs_client;
>
> nfs41_sequence_done(task, &data->res.seq_res);
>
> switch (task->tk_status) {
> case -NFS4ERR_DELAY:
> if (nfs4_async_handle_error(task, data->server, NULL,
> NULL) == -EAGAIN)
> - rpc_restart_call_prepare(task);
> + if (refcount_read(&clp->cl_count) > 1)
> + rpc_restart_call_prepare(task);
Do we really need to make the rpc restart call conditional here? Most
servers prefer that you finish freeing state before calling
DESTROY_CLIENTID.
> }
> }
>
> static void nfs41_free_stateid_release(void *calldata)
> {
> + struct nfs_free_stateid_data *data = calldata;
> + struct nfs_client *clp = data->server->nfs_client;
> +
> + nfs_put_client(clp);
> kfree(calldata);
> }
>
> @@ -10193,6 +10199,10 @@ static int nfs41_free_stateid(struct
> nfs_server *server,
> };
> struct nfs_free_stateid_data *data;
> struct rpc_task *task;
> + struct nfs_client *clp = server->nfs_client;
> +
> + if (!refcount_inc_not_zero(&clp->cl_count))
> + return -EIO;
>
> nfs4_state_protect(server->nfs_client,
> NFS_SP4_MACH_CRED_STATEID,
> &task_setup.rpc_client, &msg);
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
