Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 5 Jul 2022 09:39:33 -0300
From:   Jason Gunthorpe <jgg@ziepe.ca>
To:     Bart Van Assche <bvanassche@acm.org>
Cc:     Hillf Danton <hdanton@sina.com>,
        Mike Christie <michael.christie@oracle.com>,
        "lizhijian@fujitsu.com" <lizhijian@fujitsu.com>,
        Leon Romanovsky <leon@kernel.org>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
        "target-devel@vger.kernel.org" <target-devel@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: use-after-free in srpt_enable_tpg()
Message-ID: <20220705123933.GD23621@ziepe.ca>
References: <17649b9c-7e42-1625-8bc9-8ad333ab771c@fujitsu.com>
 <ed7e268e-94c5-38b1-286d-e2cb10412334@acm.org>
 <fbaca135-891c-7ff3-d7ac-bd79609849f5@oracle.com>
 <20220701015934.1105-1-hdanton@sina.com>
 <20220703021119.1109-1-hdanton@sina.com>
 <20220704001157.1644-1-hdanton@sina.com>
 <a671867f-153c-75a4-0f58-8dcb0d4f9c19@acm.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a671867f-153c-75a4-0f58-8dcb0d4f9c19@acm.org>
Precedence: bulk

On Mon, Jul 04, 2022 at 09:34:07PM -0700, Bart Van Assche wrote:
> On 7/3/22 17:11, Hillf Danton wrote:
> > On Sun, 3 Jul 2022 07:55:05 -0700 Bart Van Assche wrote:
> > > However, I'm not sure that would make a
> > > significant difference since there is a similar while-loop in one of the
> > > callers of srpt_remove_one() (disable_device() in the RDMA core).
> > 
> > Hehe... feel free to shed light on how the loop in RDMA core is currently
> > making the loop in srpt more prone to uaf?
> 
> In my email I was referring to the following code in disable_device():
> 
>        wait_for_completion(&device->unreg_completion);
> 
> I think that code shows that device removal by the RDMA core is synchronous
> in nature. Even if the ib_srpt source code would be modified such that the
> objects referred by that code live longer, the wait loop in disable_device()
> would wait for the ib_device reference counts to drop to zero.

That is not really the "ib_device" reference count it is the
"registration" reference count.

IB has a system where drivers/ulp can create critical regions where
the ib device must be registered using the ib_device_try_get()/put
calls. "Must be registered" is useful in a number of places but should
not be held for a long period.

This is distinct from the normal struct device refcount that simply
keeps the ib_device memory alive.

Jason