Skip to content

Commit

Permalink
iscsi-target: Always wait for kthread_should_stop() before kthread exit
Browse files Browse the repository at this point in the history
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: Jiang Yi <jiangyilism@gmail.com>
Cc: <stable@vger.kernel.org> # v3.12+
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
  • Loading branch information
Jiang Yi authored and Nicholas Bellinger committed May 31, 2017
1 parent 25cdda9 commit 5e0cf5e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 8 deletions.
30 changes: 24 additions & 6 deletions drivers/target/iscsi/iscsi_target.c
Original file line number Diff line number Diff line change
Expand Up @@ -3790,6 +3790,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.
Expand All @@ -3815,12 +3817,14 @@ int iscsi_target_tx_thread(void *arg)
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:
Expand All @@ -3830,8 +3834,13 @@ int iscsi_target_tx_thread(void *arg)
* 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;
}

Expand Down Expand Up @@ -4004,6 +4013,7 @@ int iscsi_target_rx_thread(void *arg)
{
int rc;
struct iscsi_conn *conn = arg;
bool conn_freed = false;

/*
* Allow ourselves to be interrupted by SIGINT so that a
Expand All @@ -4016,7 +4026,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->iscsit_get_rx_pdu)
return 0;
Expand All @@ -4025,7 +4035,15 @@ int iscsi_target_rx_thread(void *arg)

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;
}

Expand Down
6 changes: 5 additions & 1 deletion drivers/target/iscsi/iscsi_target_erl0.c
Original file line number Diff line number Diff line change
Expand Up @@ -930,8 +930,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);
Expand All @@ -942,6 +944,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;
}

Expand All @@ -955,4 +958,5 @@ 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;
}
2 changes: 1 addition & 1 deletion drivers/target/iscsi/iscsi_target_erl0.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ 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 *);

#endif /*** ISCSI_TARGET_ERL0_H ***/
4 changes: 4 additions & 0 deletions drivers/target/iscsi/iscsi_target_login.c
Original file line number Diff line number Diff line change
Expand Up @@ -1464,5 +1464,9 @@ int iscsi_target_login_thread(void *arg)
break;
}

while (!kthread_should_stop()) {
msleep(100);
}

return 0;
}

0 comments on commit 5e0cf5e

Please sign in to comment.