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

Hello Rihyeon,

On Wed, Aug 12, 2026 at 09:25:03PM +0900, Rihyeon Kim wrote:
> nvmf_create_ctrl() frees opts when ->create_ctrl() returns an error, so
> a transport must not free it on its own error paths.  nvme_fc_ctrl_free()
> therefore only calls nvmf_free_options() while ctrl->ctrl.opts is still
> set, and nvme_fc_init_ctrl() clears that pointer before its last put.
> 
> It only does so on the fail_ctrl: path.  When nvme_add_ctrl() fails,
> nvme_fc_init_ctrl() jumps to out_put_ctrl: instead, so
> nvme_fc_ctrl_free() still sees ctrl->ctrl.opts set and frees opts, and
> nvmf_create_ctrl() frees it again.
> 
> Reproduced with nvme-fcloop and failslab by failing the kvasprintf() in
> dev_set_name(), called from nvme_add_ctrl():
> 
>   BUG: KASAN: slab-use-after-free in nvmf_free_options+0x30/0x190
>    nvmf_free_options+0x30/0x190 drivers/nvme/host/fabrics.c:1284
>    nvmf_create_ctrl drivers/nvme/host/fabrics.c:1374 [inline]
>   Freed by task 5534:
>    nvme_fc_ctrl_free drivers/nvme/host/fc.c:2374 [inline]
>    nvme_fc_init_ctrl+0xe17/0x1450 drivers/nvme/host/fc.c:3605
> 
> Without KASAN, opts is freed twice.
> 
> nvme-tcp and nvme-rdma reach the same error path, but their free_ctrl
> only frees opts once the controller is on the global list, so they are
> not affected.
> 
> Move the clear down to out_put_ctrl:, which both error paths pass
> through.  The same injection then returns -EIO without a report.
> 
> Fixes: 1a9e218195a5 ("nvme: split device add from initialization")
> Cc: [email protected]
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=f58e57380a6083c4041d
> Suggested-by: Keith Busch <[email protected]>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Rihyeon Kim <[email protected]>
> ---

I created an alternative fix that makes fc behave the same way as
tcp/rdma/loop, i.e. derive ownership from seeing if the ctrl is on
the list or not:
https://lore.kernel.org/linux-nvme/[email protected]/T/#u

This has the advantage of avoiding the NULL pointer dereferences in
nvme_auth_free() and when accessing the sysfs attributes during teardown,
as reported by Sashiko, as ctrl->ctrl.opts now stays valid for the whole
teardown.

Please review.


Kind regards,
Niklas