Re: [PATCH] scsi: iscsi: fix possible memory leak when transport_register_device() fails

[Mike Christie]

On 11/9/22 3:24 AM, Yang Yingliang wrote:
> If transport_register_device() fails, transport_destroy_device() should
> be called to release the memory allocated in transport_setup_device().
>
> Fixes: 0896b7523026 ("[SCSI] open-iscsi/linux-iscsi-5 Initiator: Transport class update for iSCSI")
> Signed-off-by: Yang Yingliang <yangyi...@huawei.com>
> ---
> drivers/scsi/scsi_transport_iscsi.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index cd3db9684e52..88add31a56e3 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -2085,6 +2085,7 @@ int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id)
> return 0;
>
> release_dev:
> + transport_destroy_device(&session->dev);
> device_del(&session->dev);
> release_ida:
> if (session->ida_used)
> @@ -2462,6 +2463,7 @@ int iscsi_add_conn(struct iscsi_cls_conn *conn)
> if (err) {
> iscsi_cls_session_printk(KERN_ERR, session,
> "could not register transport's dev\n");
> + transport_destroy_device(&conn->dev);
> device_del(&conn->dev);
> return err;

Why doesn't transport_register_device undo what it did and call
transport_destroy_device? The callers like iscsi don't know what
was done, so it seems odd to call transport_destroy_device when
we got a failure.

[Yang Yingliang]

Yeah, it seems it's better to put the destroy() function in register(),
I will change
it and send a v2.

Thanks,
Yang
>
>
> .

[Yang Yingliang]

--
2.25.1