Re: [PATCH 1/2] iscsi_tcp: Fix NULL pointer dereference in iscsi_sw_tcp_conn_get_param()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022/3/3 23:03, Mike Christie wrote:
On 3/3/22 8:56 PM, Wenchao Hao wrote:
kernel might crash in iscsi_sw_tcp_conn_get_param() because it dereference
an invalid address.

The initialization of iscsi_conn's dd_data is after device_register() of
struct iscsi_cls_conn, so iscsi_conn's dd_data might not initialized when
iscsi_sw_tcp_conn_get_param() is called.


We are actually doing sysfs/device addition wrong.

We should be doing the 2 step setup where in step 1 we alloc/init.
When everything is allocated and initialized, then we should do
device_add which exposes us to sysfs. On the teardown side, we are
then supposed to do 2 steps where the remove function does device_del
which waits until sysfs accesses are completed. We can then tear
the structs down and free them and call device_put.


I reviewed the teardown flow of iscsi_cls_conn, it has already written as what you saied.


The exposure to NL would be similar where it goes into the wrapper
around device_add. However, see my comments on the other patch where
I don't think we can hit the bug you mention because every nl cmd
that calls into the drivers is done under the rx_queue_mutex.

I think we should separate the iscsi_create_conn function like we
do for sessions. This is going to be a little more involved because
you need to also convert iscsi_tcp_conn_setup and the drivers since
we can call into the drivers for the get_conn_param callout.
.


I hesitated about when should we call device_add(). I think there are two places to call it.

The first one is in iscsi_conn_setup(), after some initialization of conn, it keeps same with previous's implement and need not to change drivers' code. What's more, the change can fix iscsi_tcp's NULL pointer access. While this change can not make sure the LLDs related sources are already initialized when iscsi_cls_conn is exposed to sysfs. It means LLDs' callback are still responsible to check if the resources are accessible.

Another one is in create_conn callback for each driver's iscsi_transport. This need us to change each driver's code.

I send 2 patches which make changes in iscsi_conn_setup(), it's ok with iscsi_tcp, would you help to review them?




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux