Re: [PATCH -next 1/2] scsi: iscsi: fix NULL pointer dereference in iscsi_sw_tcp_release_conn()

[Mike Christie <michael.christie@oracle.com>]

On 8/21/26 4:47 AM, sashiko-bot@kernel.org wrote:
> 
>> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
>> index 8aa76f813bcdb..aea319c1e72fe 100644
>> --- a/drivers/scsi/scsi_transport_iscsi.c
>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>> @@ -2278,35 +2280,36 @@ static int iscsi_if_stop_conn(struct iscsi_cls_conn *conn, int flag)
> 
> [ ... ]
> 
>> -		/*
>> -		 * Only clear for recovery to avoid extra cleanup runs during
>> -		 * termination.
>> -		 */
>> -		spin_lock_irq(&conn->lock);
>> -		clear_bit(ISCSI_CLS_CONN_BIT_CLEANUP, &conn->flags);
>> -		spin_unlock_irq(&conn->lock);
>> -	}
>> +	} else {
>> +		iscsi_stop_conn(conn, flag);
>> +	}
>> +
>> +	spin_lock_irq(&conn->lock);
>> +	clear_bit(ISCSI_CLS_CONN_BIT_CLEANUP, &conn->flags);
>> +	spin_unlock_irq(&conn->lock);
> 
> [Severity: High]
> Does this unconditionally clear ISCSI_CLS_CONN_BIT_CLEANUP during
> STOP_CONN_TERM?
> 
> The original code avoided clearing this bit during termination, as the removed
> comment explicitly noted.
> 
> If a connection error queues iscsi_cleanup_conn_work_fn(), which unbinds the
> endpoint (setting conn->ep = NULL), and then userspace initiates termination
> via iscsi_if_stop_conn(conn, STOP_CONN_TERM), iscsi_if_disconnect_bound_ep()
> won't be called here because conn->ep is NULL.
> 
> When this bit is subsequently cleared unconditionally, and userspace later issues
> an event to disconnect the endpoint, will iscsi_if_disconnect_bound_ep() skip
> releasing the workqueue's reference?
Ye Bin, were you working on a update to this patchset? I don't think sashiko is
correct here.

Because ISCSI_CLS_CONN_BIT_CLEANUP is set we will not queue new cleanup work while
iscsi_if_stop_conn is running. Once that is done we clear the bit and if we
reconnec again the conn->state is BOUND/UP then we can set the bit again and queue
cleanup work.