Re: [PATCH rdma-next v1 2/3] RDMA/mlx5: Handling dct common resource destruction upon firmware failure

[Leon Romanovsky <leon@kernel.org>]

On Mon, Mar 20, 2023 at 04:18:14PM -0300, Jason Gunthorpe wrote:
> On Thu, Mar 16, 2023 at 03:39:27PM +0200, Leon Romanovsky wrote:
> > From: Patrisious Haddad <phaddad@nvidia.com>
> > 
> > Previously when destroying a DCT, if the firmware function for the
> > destruction failed, the common resource would have been destroyed
> > either way, since it was destroyed before the firmware object.
> > Which leads to kernel warning "refcount_t: underflow" which indicates
> > possible use-after-free.
> > Which is triggered when we try to destroy the common resource for the
> > second time and execute refcount_dec_and_test(&common->refcount).
> > 
> > So, currently before destroying the common resource we check its
> > refcount and continue with the destruction only if it isn't zero.
> 
> This seems super sketchy
> 
> If the destruction fails why not set the refcount back to 1?

Because destruction will fail in destroy_rq_tracked() which is after
destroy_resource_common().

In first destruction attempt, we delete qp from radix tree and wait for all
reference to drop. In order do not undo all this logic (setting 1 alone is
not enough), it is much safer simply skip destroy_resource_common() in reentry
case.

Failure to delete means that something external to kernel holds reference to that
QP, but it is safe to delete from kernel as nothing in kernel can use it after call
to destroy_resource_common().

Thanks

> 
> Jason