Re: [PATCH] nvme: fix admin request_queue lifetime

[Ewan Milne <emilne@redhat.com>]

On Wed, Nov 5, 2025 at 5:35 PM Keith Busch <kbusch@kernel.org> wrote:
>
> On Wed, Nov 05, 2025 at 04:31:13PM -0500, Ewan Milne wrote:
> > On Tue, Nov 4, 2025 at 6:00 PM Keith Busch <kbusch@meta.com> wrote:
> > >
> > > From: Keith Busch <kbusch@kernel.org>
> > >
> > > The namespaces can access the controller's admin request_queue, and
> > > stale references on the namespaces may exist. Ensure the request_queue
> > > is active by moving the controller's 'put' after all references on the
> > > controller have been released to ensure no one is trying to access the
> > > request_queue. This fixes a reported use-after-free bug:
> > >
> >
> > OK, so I get that this fixes the use-after-free, and don't let my
> > comments hold up
> > acceptance of the patch.  But can you explain why this actually helps?
> > nvme_alloc_admin_tag_set() allocates the admin_q as part of the admin tagset
> > initiailization, and doesn't this change keep the lifetime of the
> > admin_q past when
> > the admin tagset is deallocated?  So where do we detect that?
>
> We still call blk_mq_destroy_queue() prior to calling
> blk_mq_free_tag_set(). The queue has exited the tagset and set to dying;
> no one can "enter" the queue after that, so that tagset can be safely
> freed even if people are holding references on that dying queue.
>

OK, thanks.  I think it wise to include Chaitanya's null pointer check also.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>