Re: [PATCH V2 1/3] target: iscsi: fix hang in the iSCSI login code

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

On 4/26/23 10:35 AM, Mike Christie wrote:
> On 4/17/23 12:17 PM, Maurizio Lombardi wrote:
>>  static void iscsi_target_do_login_rx(struct work_struct *work)
>>  {
>>  	struct iscsit_conn *conn = container_of(work,
>> @@ -562,12 +543,15 @@ static void iscsi_target_do_login_rx(struct work_struct *work)
>>  	struct iscsi_np *np = login->np;
>>  	struct iscsi_portal_group *tpg = conn->tpg;
>>  	struct iscsi_tpg_np *tpg_np = conn->tpg_np;
>> -	struct conn_timeout timeout;
>>  	int rc, zero_tsih = login->zero_tsih;
>>  	bool state;
>>  
>>  	pr_debug("entering iscsi_target_do_login_rx, conn: %p, %s:%d\n",
>>  			conn, current->comm, current->pid);
>> +
>> +	spin_lock(&conn->login_worker_lock);
>> +	set_bit(LOGIN_FLAGS_WORKER_RUNNING, &conn->login_flags);
>> +	spin_unlock(&conn->login_worker_lock);
> 
> 
> I think the locking bh use is reversed.

Ignore the locking comment like here and below. I see there are 2 locks.


>> +
>> +		if (iscsi_target_sk_check_and_clear(conn, LOGIN_FLAGS_INITIAL_PDU))
>> +			ret = -1;
>> +		else if (!test_bit(LOGIN_FLAGS_WORKER_RUNNING, &conn->login_flags)) {
>> +			if (iscsit_set_login_timer_kworker(conn, NULL) < 0) {
> 
> I think this will deadlock because iscsit_set_login_timer_kworker tries to take the
> login_worker_lock we took above.