Re: [PATCH 1/4] nfs: use-after-free in svc_process_common()

[Trond Myklebust <trondmy@hammerspace.com>]

On Tue, 2018-12-18 at 23:02 +0300, Vasily Averin wrote:
> On 12/18/18 5:55 PM, Trond Myklebust wrote:
> > > > It probably also requires us to store a pointer to struct net
> > > > in
> > > > the
> > > > struct svc_rqst so that nfs4_callback_compound() and
> > > > svcauth_gss_accept() can find it, but that should be OK since
> > > > the
> > > > transport already has that referenced.
> 
> Ok, I can fix these functions and their sub-calls.
> However  rqst->rq_xprt is used in other functions that seems can be
> called inside svc_process_common() 
> - in trace_svc_process(rqstp, progp->pg_name);
> - in svc_reserve_auth(rqstp, ...) -> svc_reserve()
> - svc_authorise() -> svcauth_gss_release()
> 
> It seems I should fix these places too, it isn't?
> could you please advise how to fix svc_reserve() ?

We don't want svc_reserve() to run at all for the back channel, so I
guess that a test for rqstp->rq_xprt != NULL is appropriate there too.

svcauth_gss_release() is just using rqstp->rq_xprt to find the net
namespace, so if you add a pointer rqstp->rq_net to fix
nfs4_callback_compound, then that will fix the gss case as well.

For trace_svc_process(), maybe pull rqst->rq_xprt->xpt_remotebuf out of
the tracepoint definition in include/trace/events/sunrpc.h and make it
a tracepoint argument that is allowed to be NULL?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com