From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dominique Martinet Subject: Re: [PATCH v4] 9p: Add refcount to p9_req_t Date: Mon, 3 Sep 2018 06:36:20 +0200 Message-ID: <20180903043620.GA11460@nautica> References: <1535518779-28551-1-git-send-email-asmadeus@codewreck.org> <1535626341-20693-1-git-send-email-asmadeus@codewreck.org> <96b44210-3c4d-b5c9-0806-ad4b53fe911f@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Eric Van Hensbergen , Latchesar Ionkov , v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, syzkaller@googlegroups.com, Dominique Martinet To: Tomas Bortoli Return-path: Content-Disposition: inline In-Reply-To: <96b44210-3c4d-b5c9-0806-ad4b53fe911f@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org Tomas Bortoli wrote on Fri, Aug 31, 2018: > On 08/30/2018 12:52 PM, Dominique Martinet wrote: > > From: Tomas Bortoli > > > > 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 > > Signed-off-by: Tomas Bortoli > > Reported-by: syzbot+467050c1ce275af2a5b8@syzkaller.appspotmail.com > > Signed-off-by: Dominique Martinet > > --- > > 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