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

Rihyeon Kim <[email protected]>
Newsgroups org.infradead.lists.linux-nvme,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hello,

Thanks for the suggestion.

> Wouldn't a nicer fix be to change nvme_fc_ctrl_free() to look more like
> nvme_tcp_ctrl_free(), i.e. something like:
>
> +       if (list_empty(&ctrl->ctrl_list))
> +               goto free_ctrl;

I am not sure whether that would work, and I may well be missing
something.  From what I could tell, nvme_tcp_create_ctrl() does its
list_add_tail() last, while nvme_fc_init_ctrl() does it before the
nvme_change_ctrl_state() and queue_delayed_work() checks, so on those two
failure paths the controller is already on the list and opts would end up
freed twice again.  The early goto would also skip the ida_free(),
put_device() and nvme_fc_rport_put() for what nvme_fc_alloc_ctrl() takes
before the list_add.

I am not familiar with this driver either, so please correct me if I have
misread it.

Keith suggested moving the existing clear from fail_ctrl: down to
out_put_ctrl: instead, so I will send that as v2.

Thanks,
Rihyeon
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.