Re: [PATCH] nvme-fc: fix double free of fabrics options when nvme_add_ctrl() fails

[Keith Busch <kbusch@kernel.org>]

On Fri, Aug 21, 2026 at 03:21:56PM +0200, Niklas Cassel wrote:
> On Thu, Aug 20, 2026 at 09:23:10AM -0600, Keith Busch wrote:
> > On Wed, Aug 19, 2026 at 08:32:33PM +0200, Niklas Cassel wrote:
> > > 
> > > I am not following.
> > > 
> > > The options is freed in two different places, depending on if the
> > > ops->create_ctrl(dev, opts) call in fabrics.c:nvmf_create_ctrl() was
> > > successful or not.
> > 
> > Oh, not that path. You mentioned previously that ctrl->ops is
> > dereferenced in other places. I thought this means we can successfully
> > add the controller, which means the attributes are visible, but then
> > fail something else later that causes the opts to free. Since the
> > attribute was visible, something can open it before the device exporting
> > is torn down, and access it after the opts were freed.
> 
> Sashiko flagged another NULL pointer defererence from the sysfs attributes:
> 
> "While the sysfs attributes are still fully accessible, an unprivileged user
> could concurrently read an attribute like hostnqn, causing a NULL pointer
> dereference.":
> https://sashiko.dev/#/patchset/20260811125310.165487-1-rihyeon8648%40gmail.com
> 
> This comment was on Rihyeon Kim's patch proposal.
> 
> I assume that this is because the fc.c driver (both before and after
> Rihyeon patch) force set ctrl->ops = NULL before nvme_free_ctrl() was
> called, so a user reading the sysfs attributes at the same time as the
> controller was tearing down, could most likely crash the kernel.
> (Note that neither rdma.c,tcp.c,loop.c does this, so this problem does
> not exist for them.)
> 
> sysfs_remove_link() is called before ctrl->ops->free_ctrl(ctrl);

When ops->create_ctrl() fails, the opts are freed from
nvmf_create_ctrl(). We can still fail ops->create_ctrl() after a
successful nvme_add_ctrl(), so the handle will be visible to user space
for a moment.

I think you're relying on nvme_fc_init_ctrl's nvme_put_ctrl() error case
to be the final reference, but it might not be if the character device
was created and someone opened it. If so then:

  out_put_ctrl:
     nvme_put_ctrl()   -> refcount 1, NOT zero

So now when nvme_free_ctrl is called sometime after nvmf_create_ctrl()
freed the opts, the ctrl is pointing to freed memory.