Commit 2b12d398 authored by Jiang Yi's avatar Jiang Yi Committed by Ben Hutchings

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

commit 5e0cf5e6 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>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent 32291df8
...@@ -3931,6 +3931,8 @@ int iscsi_target_tx_thread(void *arg) ...@@ -3931,6 +3931,8 @@ int iscsi_target_tx_thread(void *arg)
{ {
int ret = 0; int ret = 0;
struct iscsi_conn *conn = arg; struct iscsi_conn *conn = arg;
bool conn_freed = false;
/* /*
* Allow ourselves to be interrupted by SIGINT so that a * Allow ourselves to be interrupted by SIGINT so that a
* connection recovery / failure event can be triggered externally. * connection recovery / failure event can be triggered externally.
...@@ -3956,13 +3958,15 @@ int iscsi_target_tx_thread(void *arg) ...@@ -3956,13 +3958,15 @@ int iscsi_target_tx_thread(void *arg)
goto transport_err; goto transport_err;
ret = iscsit_handle_response_queue(conn); ret = iscsit_handle_response_queue(conn);
if (ret == 1) if (ret == 1) {
goto get_immediate; goto get_immediate;
else if (ret == -ECONNRESET) } else if (ret == -ECONNRESET) {
conn_freed = true;
goto out; goto out;
else if (ret < 0) } else if (ret < 0) {
goto transport_err; goto transport_err;
} }
}
transport_err: transport_err:
/* /*
...@@ -3971,8 +3975,13 @@ int iscsi_target_tx_thread(void *arg) ...@@ -3971,8 +3975,13 @@ int iscsi_target_tx_thread(void *arg)
* responsible for cleaning up the early connection failure. * responsible for cleaning up the early connection failure.
*/ */
if (conn->conn_state != TARG_CONN_STATE_IN_LOGIN) 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: out:
if (!conn_freed) {
while (!kthread_should_stop()) {
msleep(100);
}
}
return 0; return 0;
} }
...@@ -4073,6 +4082,8 @@ int iscsi_target_rx_thread(void *arg) ...@@ -4073,6 +4082,8 @@ int iscsi_target_rx_thread(void *arg)
u32 checksum = 0, digest = 0; u32 checksum = 0, digest = 0;
struct iscsi_conn *conn = arg; struct iscsi_conn *conn = arg;
struct kvec iov; struct kvec iov;
bool conn_freed = false;
/* /*
* Allow ourselves to be interrupted by SIGINT so that a * Allow ourselves to be interrupted by SIGINT so that a
* connection recovery / failure event can be triggered externally. * connection recovery / failure event can be triggered externally.
...@@ -4084,7 +4095,7 @@ int iscsi_target_rx_thread(void *arg) ...@@ -4084,7 +4095,7 @@ int iscsi_target_rx_thread(void *arg)
*/ */
rc = wait_for_completion_interruptible(&conn->rx_login_comp); rc = wait_for_completion_interruptible(&conn->rx_login_comp);
if (rc < 0 || iscsi_target_check_conn_state(conn)) if (rc < 0 || iscsi_target_check_conn_state(conn))
return 0; goto out;
if (conn->conn_transport->transport_type == ISCSI_INFINIBAND) { if (conn->conn_transport->transport_type == ISCSI_INFINIBAND) {
struct completion comp; struct completion comp;
...@@ -4169,7 +4180,15 @@ int iscsi_target_rx_thread(void *arg) ...@@ -4169,7 +4180,15 @@ int iscsi_target_rx_thread(void *arg)
transport_err: transport_err:
if (!signal_pending(current)) if (!signal_pending(current))
atomic_set(&conn->transport_failed, 1); 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; return 0;
} }
......
...@@ -935,8 +935,10 @@ static void iscsit_handle_connection_cleanup(struct iscsi_conn *conn) ...@@ -935,8 +935,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); spin_lock_bh(&conn->state_lock);
if (atomic_read(&conn->connection_exit)) { if (atomic_read(&conn->connection_exit)) {
spin_unlock_bh(&conn->state_lock); spin_unlock_bh(&conn->state_lock);
...@@ -947,6 +949,7 @@ void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn) ...@@ -947,6 +949,7 @@ void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn)
if (conn->conn_state == TARG_CONN_STATE_IN_LOGOUT) { if (conn->conn_state == TARG_CONN_STATE_IN_LOGOUT) {
spin_unlock_bh(&conn->state_lock); spin_unlock_bh(&conn->state_lock);
iscsit_close_connection(conn); iscsit_close_connection(conn);
*conn_freed = true;
return; return;
} }
...@@ -960,6 +963,7 @@ void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn) ...@@ -960,6 +963,7 @@ void iscsit_take_action_for_connection_exit(struct iscsi_conn *conn)
spin_unlock_bh(&conn->state_lock); spin_unlock_bh(&conn->state_lock);
iscsit_handle_connection_cleanup(conn); iscsit_handle_connection_cleanup(conn);
*conn_freed = true;
} }
/* /*
......
...@@ -9,7 +9,7 @@ extern int iscsit_stop_time2retain_timer(struct iscsi_session *); ...@@ -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_connection_reinstatement_rcfr(struct iscsi_conn *);
extern void iscsit_cause_connection_reinstatement(struct iscsi_conn *, int); extern void iscsit_cause_connection_reinstatement(struct iscsi_conn *, int);
extern void iscsit_fall_back_to_erl0(struct iscsi_session *); 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 *); extern int iscsit_recover_from_unknown_opcode(struct iscsi_conn *);
#endif /*** ISCSI_TARGET_ERL0_H ***/ #endif /*** ISCSI_TARGET_ERL0_H ***/
...@@ -1496,5 +1496,9 @@ int iscsi_target_login_thread(void *arg) ...@@ -1496,5 +1496,9 @@ int iscsi_target_login_thread(void *arg)
break; break;
} }
while (!kthread_should_stop()) {
msleep(100);
}
return 0; return 0;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment