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

[Niklas Cassel <cassel@kernel.org>]

On Tue, Aug 11, 2026 at 09:53:10PM +0900, Rihyeon Kim wrote:

(snip)

> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 04363b9c4489..e4d0eeccd846 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -3601,6 +3601,9 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
>  	nvme_uninit_ctrl(&ctrl->ctrl);
>  
>  out_put_ctrl:
> +	/* nvme_add_ctrl() failures skip the clear in fail_ctrl: above */
> +	ctrl->ctrl.opts = NULL;
> +
>  	/* Remove core ctrl ref. */
>  	nvme_put_ctrl(&ctrl->ctrl);
>  
> 
> base-commit: 0ce37745d4bfbc493f718169c3974898ffec8ee7
> -- 
> 2.43.0
> 
> 

Wouldn't a nicer fix be to change nvme_fc_ctrl_free() to look more like
nvme_tcp_ctrl_free(), i.e. something like:

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 04363b9c4489..2b4c222995ea 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2359,19 +2359,22 @@ nvme_fc_ctrl_free(struct kref *ref)
                container_of(ref, struct nvme_fc_ctrl, ref);
        unsigned long flags;

+       if (list_empty(&ctrl->ctrl_list))
+               goto free_ctrl;
+
        /* remove from rport list */
        spin_lock_irqsave(&ctrl->rport->lock, flags);
        list_del(&ctrl->ctrl_list);
        spin_unlock_irqrestore(&ctrl->rport->lock, flags);

-       kfree(ctrl->queues);
-
        put_device(ctrl->dev);
        nvme_fc_rport_put(ctrl->rport);

        ida_free(&nvme_fc_ctrl_cnt, ctrl->cnum);
-       if (ctrl->ctrl.opts)
-               nvmf_free_options(ctrl->ctrl.opts);
+
+       nvmf_free_options(ctrl->ctrl.opts);
+free_ctrl:
+       kfree(ctrl->queues);
        kfree(ctrl);
 }

@@ -3593,8 +3596,6 @@ 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 */



However, I'm not familar with the fc driver, so this suggestion is untested.


Kind regards,
Niklas