Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d18dfb50 authored by Jiang Yi's avatar Jiang Yi Committed by Greg Kroah-Hartman
Browse files

iscsi-target: Always wait for kthread_should_stop() before kthread exit 



commit 5e0cf5e6c43b9e19fc0284f69e5cd2b4a47523b0 upstream.

There are three timing problems in the kthread usages of iscsi_target_mod:

 - np_thread of struct iscsi_np
 - rx_thread and tx_thread of struct iscsi_conn

In iscsit_close_connection(), it calls

 send_sig(SIGINT, conn->tx_thread, 1);
 kthread_stop(conn->tx_thread);

In conn->tx_thread, which is iscsi_target_tx_thread(), when it receive
SIGINT the kthread will exit without checking the return value of
kthread_should_stop().

So if iscsi_target_tx_thread() exit right between send_sig(SIGINT...)
and kthread_stop(...), the kthread_stop() will try to stop an already
stopped kthread.

This is invalid according to the documentation of kthread_stop().

(Fix -ECONNRESET logout handling in iscsi_target_tx_thread and
 early iscsi_target_rx_thread failure case - nab)

Signed-off-by: default avatarJiang Yi <jiangyilism@gmail.com>
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 8adc6142

drivers/target/iscsi/iscsi_target.c

+22 −6
Original line number Diff line number Diff line
@@ -3931,6 +3931,8 @@ int iscsi_target_tx_thread(void *arg) 
{

	int ret = 0;

	struct iscsi_conn *conn = arg;

	bool conn_freed = false;



	/*

	 * Allow ourselves to be interrupted by SIGINT so that a

	 * connection recovery / failure event can be triggered externally.
@@ -3956,13 +3958,15 @@ get_immediate: 
			goto transport_err;



		ret = iscsit_handle_response_queue(conn);

		if (ret == 1)

		if (ret == 1) {

			goto get_immediate;

		else if (ret == -ECONNRESET)

		} else if (ret == -ECONNRESET) {

			conn_freed = true;

			goto out;

		else if (ret < 0)

		} else if (ret < 0) {

			goto transport_err;

		}

	}



transport_err:

	/*
@@ -3971,8 +3975,13 @@ transport_err: 
	 * responsible for cleaning up the early connection failure.

	 */

	if (conn->conn_state != TARG_CONN_STATE_IN_LOGIN)

		iscsit_take_action_for_connection_exit(conn);

		iscsit_take_action_for_connection_exit(conn, &conn_freed);

out:

	if (!conn_freed) {

		while (!kthread_should_stop()) {

			msleep(100);

		}

	}

	return 0;

}
@@ -4073,6 +4082,7 @@ int iscsi_target_rx_thread(void *arg) 
	u32 checksum = 0, digest = 0;

	struct iscsi_conn *conn = arg;

	struct kvec iov;

	bool conn_freed = false;

	/*

	 * Allow ourselves to be interrupted by SIGINT so that a

	 * connection recovery / failure event can be triggered externally.
@@ -4084,7 +4094,7 @@ int iscsi_target_rx_thread(void *arg) 
	 */

	rc = wait_for_completion_interruptible(&conn->rx_login_comp);

	if (rc < 0 || iscsi_target_check_conn_state(conn))

		return 0;

		goto out;



	if (conn->conn_transport->transport_type == ISCSI_INFINIBAND) {

		struct completion comp;
@@ -4169,7 +4179,13 @@ int iscsi_target_rx_thread(void *arg) 
transport_err:

	if (!signal_pending(current))

		atomic_set(&conn->transport_failed, 1);

	iscsit_take_action_for_connection_exit(conn);

	iscsit_take_action_for_connection_exit(conn, &conn_freed);

out:

	if (!conn_freed) {

		while (!kthread_should_stop()) {

			msleep(100);

		}

	}

	return 0;

}

drivers/target/iscsi/iscsi_target_erl0.c

+5 −1
Original line number Diff line number Diff line
@@ -931,8 +931,10 @@ static void iscsit_handle_connection_cleanup(struct iscsi_conn *conn) 
	}

}



void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn)

void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn, bool *conn_freed)

{

	*conn_freed = false;



	spin_lock_bh(&conn->state_lock);

	if (atomic_read(&conn->connection_exit)) {

		spin_unlock_bh(&conn->state_lock);
@@ -943,6 +945,7 @@ void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn) 
	if (conn->conn_state == TARG_CONN_STATE_IN_LOGOUT) {

		spin_unlock_bh(&conn->state_lock);

		iscsit_close_connection(conn);

		*conn_freed = true;

		return;

	}
@@ -956,6 +959,7 @@ void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn) 
	spin_unlock_bh(&conn->state_lock);



	iscsit_handle_connection_cleanup(conn);

	*conn_freed = true;

}



/*

drivers/target/iscsi/iscsi_target_erl0.h

+1 −1
Original line number Diff line number Diff line
@@ -9,7 +9,7 @@ extern int iscsit_stop_time2retain_timer(struct iscsi_session *); 
extern void iscsit_connection_reinstatement_rcfr(struct iscsi_conn *);

extern void iscsit_cause_connection_reinstatement(struct iscsi_conn *, int);

extern void iscsit_fall_back_to_erl0(struct iscsi_session *);

extern void iscsit_take_action_for_connection_exit(struct iscsi_conn *);

extern void iscsit_take_action_for_connection_exit(struct iscsi_conn *, bool *);

extern int iscsit_recover_from_unknown_opcode(struct iscsi_conn *);



#endif   /*** ISCSI_TARGET_ERL0_H ***/

drivers/target/iscsi/iscsi_target_login.c

+4 −0
Original line number Diff line number Diff line
@@ -1492,5 +1492,9 @@ int iscsi_target_login_thread(void *arg) 
			break;

	}



	while (!kthread_should_stop()) {

		msleep(100);

	}



	return 0;

}