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

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

[Rihyeon Kim]

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: stable@vger.kernel.org
Reported-by: syzbot+f58e57380a6083c4041d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f58e57380a6083c4041d
Suggested-by: Keith Busch <kbusch@kernel.org>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Rihyeon Kim <rihyeon8648@gmail.com>
---
Changes since v1: move the existing clear from fail_ctrl: down to
out_put_ctrl: rather than adding a second one (Keith).  Re-tested by
sweeping fail-nth 1..200 over the connect write with fcloop and failslab,
which reaches both labels: unpatched reports at 17, this passes 200/200.

v1: https://lore.kernel.org/linux-nvme/20260811125310.165487-1-rihyeon8648@gmail.com/

 drivers/nvme/host/fc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 04363b9c4489..a35ed9c8c212 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3593,14 +3593,15 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 	cancel_work_sync(&ctrl->ctrl.reset_work);
 	cancel_delayed_work_sync(&ctrl->connect_work);
 
-	ctrl->ctrl.opts = NULL;
-
 	if (ctrl->ctrl.admin_tagset)
 		nvme_remove_admin_tag_set(&ctrl->ctrl);
 	/* initiate nvme ctrl ref counting teardown */
 	nvme_uninit_ctrl(&ctrl->ctrl);
 
 out_put_ctrl:
+	/* the caller frees opts when we return an error */
+	ctrl->ctrl.opts = NULL;
+
 	/* Remove core ctrl ref. */
 	nvme_put_ctrl(&ctrl->ctrl);
 

base-commit: 0ce37745d4bfbc493f718169c3974898ffec8ee7
-- 
2.43.0

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

[Niklas Cassel]

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: stable@vger.kernel.org
> Reported-by: syzbot+f58e57380a6083c4041d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f58e57380a6083c4041d
> Suggested-by: Keith Busch <kbusch@kernel.org>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Rihyeon Kim <rihyeon8648@gmail.com>
> ---

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/20260814143833.1953415-2-cassel@kernel.org/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