[0/2] scsi:libiscsi: Add iscsi_cls_conn device to sysfs correctly

Message

Wenchao Hao March 8, 2022, 12:56 a.m. UTC

We found a NULL pointer dereference in iscsi_sw_tcp_conn_get_param(),
the root reason is we did sysfs addition wrong.

iscsi_create_conn() expose iscsi_cls_conn to sysfs while the related
resources are not initialized. So we should delay the calling of
device_add() until these resources has been initialized.

This patchset solve this issue by changing iscsi_conn_setup() and works 
well for iscsi_tcp.

Wenchao Hao (2):
  scsi: iscsi: Add helper functions to alloc and add iscsi_cls_conn
  scsi:libiscsi: Add iscsi_cls_conn to sysfs after been initialized

 drivers/scsi/libiscsi.c             | 13 ++++-
 drivers/scsi/scsi_transport_iscsi.c | 85 +++++++++++++++++++++++------
 include/scsi/scsi_transport_iscsi.h |  3 +
 3 files changed, 83 insertions(+), 18 deletions(-)

Comments

Mike Christie March 7, 2022, 4:12 p.m. UTC | #1

On 3/7/22 6:56 PM, Wenchao Hao wrote:
> We found a NULL pointer dereference in iscsi_sw_tcp_conn_get_param(),
> the root reason is we did sysfs addition wrong.
> 
> iscsi_create_conn() expose iscsi_cls_conn to sysfs while the related
> resources are not initialized. So we should delay the calling of
> device_add() until these resources has been initialized.
> 
> This patchset solve this issue by changing iscsi_conn_setup() and works 
> well for iscsi_tcp.
> 

Overall I think you need to also fix up the drivers. It just makes it a
nicer driver API where the LLDs don't know about sysfs and doesn't have
to worry about it.

Let's start with just this first piece where we handle sysfs in the lib
and class like you are doing in this patchset. We can do the LLDs
interaction with the lib in a second patchset to make this easier and fix
the initial bug and cleanup some code.

In a separate patchset, we can then go deeper and maybe just merge/kill some
of the lib/class interface since every driver except qla4xxx hooks into the
lib. So we have this distinction just for that one driver's session mode
and that doesn't make a lot of sense.