On Fri, 2017-03-10 at 16:56 -0500, Olga Kornievskaia wrote:
> On Fri, Mar 10, 2017 at 4:35 PM, Olga Kornievskaia <kolga@xxxxxxxxxx>
> wrote:
> > Since rpc_task is async, the release function should be called
> > which
> > will free the impl_id, scope, and owner.
> >
> > Trond pointed at 2 more problems:
> > -- use of client pointer after free in the
> > nfs4_exchangeid_release() function
> > -- cl_count mismatch if rpc_run_task() isn't run
> >
> > Fixes: 8d89bd70bc9 ("NFS setup async exchange_id")
> > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> > ---
> > fs/nfs/nfs4proc.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 59be0f7..3a79d3a 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -7426,11 +7426,11 @@ static void nfs4_exchange_id_release(void
> > *data)
> > struct nfs41_exchange_id_data *cdata =
> > (struct
> > nfs41_exchange_id_data *)data;
> >
> > - nfs_put_client(cdata->args.client);
> > if (cdata->xprt) {
> > xprt_put(cdata->xprt);
> > rpc_clnt_xprt_switch_put(cdata->args.client-
> > >cl_rpcclient);
> > }
> > + nfs_put_client(cdata->args.client);
> > kfree(cdata->res.impl_id);
> > kfree(cdata->res.server_scope);
> > kfree(cdata->res.server_owner);
> > @@ -7537,10 +7537,8 @@ static int _nfs4_proc_exchange_id(struct
> > nfs_client *clp, struct rpc_cred *cred,
> > task_setup_data.callback_data = calldata;
> >
> > task = rpc_run_task(&task_setup_data);
> > - if (IS_ERR(task)) {
> > - status = PTR_ERR(task);
> > - goto out_impl_id;
> > - }
> > + if (IS_ERR(task))
> > + return PTR_ERR(task);
> >
> > if (!xprt) {
> > status = rpc_wait_for_completion_task(task);
> > @@ -7558,6 +7556,8 @@ static int _nfs4_proc_exchange_id(struct
> > nfs_client *clp, struct rpc_cred *cred,
> > clp->cl_implid->date.seconds,
> > clp->cl_implid->date.nseconds);
> > dprintk("NFS reply exchange_id: %d\n", status);
> > + if (status)
> > + nfs_put_client(clp);
This needs to be in the "out_calldata" error path only. It isn't needed
once we've called rpc_run_task(). Otherwise the patch looks good.
> > return status;
> >
> > out_impl_id:
>
> Urgh. scratch this one, it's causing problems. Will try again.
>
>
> > --
> > 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
>
>
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
