Re: [PATCH v4] 9p: Add refcount to p9_req_t

[Dominique Martinet]

Tomas Bortoli wrote on Fri, Aug 31, 2018:
> On 08/30/2018 12:52 PM, Dominique Martinet wrote:
> > From: Tomas Bortoli <tomasbortoli@xxxxxxxxx>
> > 
> > To avoid use-after-free(s), use a refcount to keep track of the
> > usable references to any instantiated struct p9_req_t.
> > 
> > This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
> > wrappers to kref_put(), kref_get() and kref_get_unless_zero().
> > These are used by the client and the transports to keep track of
> > valid requests' references.
> > 
> > p9_free_req() is added back and used as callback by kref_put().
> > 
> > Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
> > kmem_cache_free() will not be reused for another type until the rcu
> > synchronisation period is over, so an address gotten under rcu read
> > lock is safe to inc_ref() without corrupting random memory while
> > the lock is held.
> > 
> > Co-developed-by: Dominique Martinet <dominique.martinet@xxxxxx>
> > Signed-off-by: Tomas Bortoli <tomasbortoli@xxxxxxxxx>
> > Reported-by: syzbot+467050c1ce275af2a5b8@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Dominique Martinet <dominique.martinet@xxxxxx>
> > ---
> > v3:
> >  - add req put if virtio zc request fails
> >  - add req put if cancelled callback is not defined for virtio
> >  - (incorrectly) add req put in rdma cancelled callback
> > 
> > v4:
> >  - removed rdma's cancelled callback put again
> >  - changed the else if no cancelled callback into actually giving virtio
> > a callback, xen does not need to call put in that case either because
> > both function rely on tag_lookup to find the request. trans_fd only
> > needs to put in cancelled because it also keeps the req in a list around
> > for cancel.
> >  - add req put for trans xen's request(), I'm not sure why that one was
> > missing either..
> > 
> > And with that I believe I am done testing all four transports.
> > I'll do a second round of tests next week just to make sure, but it
> > should be good enoughâ
> > Sorry for the multiple iterations.
>
> LGTM, thanks Dominique!

Thanks.

I've pushed this with the other patches to my '9p-next' branch, which
will get merged to linux-next today/tomorrow, so they can soak up some
syzbot testing as well.
That doesn't mean they cannot get reviews anymore, so don't be shy!


Tomas, I didn't see you reply about the 'rename req to rreq' requested
patch for trans_fd, but it's trivial so if you're not going to do it I
will submit something around next week.

-- 
Dominique