| Message ID | 20221109092421.3111613-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: iscsi: fix possible memory leak when transport_register_device() fails | expand |
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 <yangyingliang@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.
On 2022/11/10 2:51, Mike Christie wrote: > 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 <yangyingliang@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. Yeah, it seems it's better to put the destroy() function in register(), I will change it and send a v2. Thanks, Yang > > > .
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; }
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 <yangyingliang@huawei.com> --- drivers/scsi/scsi_transport_iscsi.c | 2 ++ 1 file changed, 2 insertions(+)