Skip to content

Commit e541986

Browse files
author
Nicholas Bellinger
committed
iscsi-target: Fix iscsit_start_kthreads failure OOPs
This patch fixes a regression introduced with the following commit in v4.0-rc1 code, where a iscsit_start_kthreads() failure triggers a NULL pointer dereference OOPs: commit 88dcd2d Author: Nicholas Bellinger <nab@linux-iscsi.org> Date: Thu Feb 26 22:19:15 2015 -0800 iscsi-target: Convert iscsi_thread_set usage to kthread.h To address this bug, move iscsit_start_kthreads() immediately preceeding the transmit of last login response, before signaling a successful transition into full-feature-phase within existing iscsi_target_do_tx_login_io() logic. This ensures that no target-side resource allocation failures can occur after the final login response has been successfully sent. Also, it adds a iscsi_conn->rx_login_comp to allow the RX thread to sleep to prevent other socket related failures until the final iscsi_post_login_handler() call is able to complete. Cc: Sagi Grimberg <sagig@mellanox.com> Cc: <stable@vger.kernel.org> # v3.10+ Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
1 parent 417c20a commit e541986

File tree

5 files changed

+68
-33
lines changed

5 files changed

+68
-33
lines changed

‎drivers/target/iscsi/iscsi_target.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3998,7 +3998,13 @@ int iscsi_target_tx_thread(void *arg)
39983998
}
39993999

40004000
transport_err:
4001-
iscsit_take_action_for_connection_exit(conn);
4001+
/*
4002+
* Avoid the normal connection failure code-path if this connection
4003+
* is still within LOGIN mode, and iscsi_np process context is
4004+
* responsible for cleaning up the early connection failure.
4005+
*/
4006+
if (conn->conn_state != TARG_CONN_STATE_IN_LOGIN)
4007+
iscsit_take_action_for_connection_exit(conn);
40024008
out:
40034009
return 0;
40044010
}
@@ -4082,7 +4088,7 @@ static int iscsi_target_rx_opcode(struct iscsi_conn *conn, unsigned char *buf)
40824088

40834089
int iscsi_target_rx_thread(void *arg)
40844090
{
4085-
int ret;
4091+
int ret, rc;
40864092
u8 buffer[ISCSI_HDR_LEN], opcode;
40874093
u32 checksum = 0, digest = 0;
40884094
struct iscsi_conn *conn = arg;
@@ -4092,10 +4098,16 @@ int iscsi_target_rx_thread(void *arg)
40924098
* connection recovery / failure event can be triggered externally.
40934099
*/
40944100
allow_signal(SIGINT);
4101+
/*
4102+
* Wait for iscsi_post_login_handler() to complete before allowing
4103+
* incoming iscsi/tcp socket I/O, and/or failing the connection.
4104+
*/
4105+
rc = wait_for_completion_interruptible(&conn->rx_login_comp);
4106+
if (rc < 0)
4107+
return 0;
40954108

40964109
if (conn->conn_transport->transport_type == ISCSI_INFINIBAND) {
40974110
struct completion comp;
4098-
int rc;
40994111

41004112
init_completion(&comp);
41014113
rc = wait_for_completion_interruptible(&comp);

‎drivers/target/iscsi/iscsi_target_login.c

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ static struct iscsi_login *iscsi_login_init_conn(struct iscsi_conn *conn)
8282
init_completion(&conn->conn_logout_comp);
8383
init_completion(&conn->rx_half_close_comp);
8484
init_completion(&conn->tx_half_close_comp);
85+
init_completion(&conn->rx_login_comp);
8586
spin_lock_init(&conn->cmd_lock);
8687
spin_lock_init(&conn->conn_usage_lock);
8788
spin_lock_init(&conn->immed_queue_lock);
@@ -644,7 +645,7 @@ static void iscsi_post_login_start_timers(struct iscsi_conn *conn)
644645
iscsit_start_nopin_timer(conn);
645646
}
646647

647-
static int iscsit_start_kthreads(struct iscsi_conn *conn)
648+
int iscsit_start_kthreads(struct iscsi_conn *conn)
648649
{
649650
int ret = 0;
650651

@@ -679,6 +680,7 @@ static int iscsit_start_kthreads(struct iscsi_conn *conn)
679680

680681
return 0;
681682
out_tx:
683+
send_sig(SIGINT, conn->tx_thread, 1);
682684
kthread_stop(conn->tx_thread);
683685
conn->tx_thread_active = false;
684686
out_bitmap:
@@ -689,7 +691,7 @@ static int iscsit_start_kthreads(struct iscsi_conn *conn)
689691
return ret;
690692
}
691693

692-
int iscsi_post_login_handler(
694+
void iscsi_post_login_handler(
693695
struct iscsi_np *np,
694696
struct iscsi_conn *conn,
695697
u8 zero_tsih)
@@ -699,7 +701,6 @@ int iscsi_post_login_handler(
699701
struct se_session *se_sess = sess->se_sess;
700702
struct iscsi_portal_group *tpg = sess->tpg;
701703
struct se_portal_group *se_tpg = &tpg->tpg_se_tpg;
702-
int rc;
703704

704705
iscsit_inc_conn_usage_count(conn);
705706

@@ -739,10 +740,6 @@ int iscsi_post_login_handler(
739740
sess->sess_ops->InitiatorName);
740741
spin_unlock_bh(&sess->conn_lock);
741742

742-
rc = iscsit_start_kthreads(conn);
743-
if (rc)
744-
return rc;
745-
746743
iscsi_post_login_start_timers(conn);
747744
/*
748745
* Determine CPU mask to ensure connection's RX and TX kthreads
@@ -751,15 +748,20 @@ int iscsi_post_login_handler(
751748
iscsit_thread_get_cpumask(conn);
752749
conn->conn_rx_reset_cpumask = 1;
753750
conn->conn_tx_reset_cpumask = 1;
754-
751+
/*
752+
* Wakeup the sleeping iscsi_target_rx_thread() now that
753+
* iscsi_conn is in TARG_CONN_STATE_LOGGED_IN state.
754+
*/
755+
complete(&conn->rx_login_comp);
755756
iscsit_dec_conn_usage_count(conn);
757+
756758
if (stop_timer) {
757759
spin_lock_bh(&se_tpg->session_lock);
758760
iscsit_stop_time2retain_timer(sess);
759761
spin_unlock_bh(&se_tpg->session_lock);
760762
}
761763
iscsit_dec_session_usage_count(sess);
762-
return 0;
764+
return;
763765
}
764766

765767
iscsi_set_session_parameters(sess->sess_ops, conn->param_list, 1);
@@ -800,10 +802,6 @@ int iscsi_post_login_handler(
800802
" iSCSI Target Portal Group: %hu\n", tpg->nsessions, tpg->tpgt);
801803
spin_unlock_bh(&se_tpg->session_lock);
802804

803-
rc = iscsit_start_kthreads(conn);
804-
if (rc)
805-
return rc;
806-
807805
iscsi_post_login_start_timers(conn);
808806
/*
809807
* Determine CPU mask to ensure connection's RX and TX kthreads
@@ -812,10 +810,12 @@ int iscsi_post_login_handler(
812810
iscsit_thread_get_cpumask(conn);
813811
conn->conn_rx_reset_cpumask = 1;
814812
conn->conn_tx_reset_cpumask = 1;
815-
813+
/*
814+
* Wakeup the sleeping iscsi_target_rx_thread() now that
815+
* iscsi_conn is in TARG_CONN_STATE_LOGGED_IN state.
816+
*/
817+
complete(&conn->rx_login_comp);
816818
iscsit_dec_conn_usage_count(conn);
817-
818-
return 0;
819819
}
820820

821821
static void iscsi_handle_login_thread_timeout(unsigned long data)
@@ -1380,23 +1380,12 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
13801380
if (ret < 0)
13811381
goto new_sess_out;
13821382

1383-
if (!conn->sess) {
1384-
pr_err("struct iscsi_conn session pointer is NULL!\n");
1385-
goto new_sess_out;
1386-
}
1387-
13881383
iscsi_stop_login_thread_timer(np);
13891384

1390-
if (signal_pending(current))
1391-
goto new_sess_out;
1392-
13931385
if (ret == 1) {
13941386
tpg_np = conn->tpg_np;
13951387

1396-
ret = iscsi_post_login_handler(np, conn, zero_tsih);
1397-
if (ret < 0)
1398-
goto new_sess_out;
1399-
1388+
iscsi_post_login_handler(np, conn, zero_tsih);
14001389
iscsit_deaccess_np(np, tpg, tpg_np);
14011390
}
14021391

‎drivers/target/iscsi/iscsi_target_login.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ extern int iscsit_accept_np(struct iscsi_np *, struct iscsi_conn *);
1212
extern int iscsit_get_login_rx(struct iscsi_conn *, struct iscsi_login *);
1313
extern int iscsit_put_login_tx(struct iscsi_conn *, struct iscsi_login *, u32);
1414
extern void iscsit_free_conn(struct iscsi_np *, struct iscsi_conn *);
15-
extern int iscsi_post_login_handler(struct iscsi_np *, struct iscsi_conn *, u8);
15+
extern int iscsit_start_kthreads(struct iscsi_conn *);
16+
extern void iscsi_post_login_handler(struct iscsi_np *, struct iscsi_conn *, u8);
1617
extern void iscsi_target_login_sess_out(struct iscsi_conn *, struct iscsi_np *,
1718
bool, bool);
1819
extern int iscsi_target_login_thread(void *);

‎drivers/target/iscsi/iscsi_target_nego.c

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
******************************************************************************/
1818

1919
#include <linux/ctype.h>
20+
#include <linux/kthread.h>
2021
#include <scsi/iscsi_proto.h>
2122
#include <target/target_core_base.h>
2223
#include <target/target_core_fabric.h>
@@ -361,10 +362,24 @@ static int iscsi_target_do_tx_login_io(struct iscsi_conn *conn, struct iscsi_log
361362
ntohl(login_rsp->statsn), login->rsp_length);
362363

363364
padding = ((-login->rsp_length) & 3);
365+
/*
366+
* Before sending the last login response containing the transition
367+
* bit for full-feature-phase, go ahead and start up TX/RX threads
368+
* now to avoid potential resource allocation failures after the
369+
* final login response has been sent.
370+
*/
371+
if (login->login_complete) {
372+
int rc = iscsit_start_kthreads(conn);
373+
if (rc) {
374+
iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
375+
ISCSI_LOGIN_STATUS_NO_RESOURCES);
376+
return -1;
377+
}
378+
}
364379

365380
if (conn->conn_transport->iscsit_put_login_tx(conn, login,
366381
login->rsp_length + padding) < 0)
367-
return -1;
382+
goto err;
368383

369384
login->rsp_length = 0;
370385
mutex_lock(&sess->cmdsn_mutex);
@@ -373,6 +388,23 @@ static int iscsi_target_do_tx_login_io(struct iscsi_conn *conn, struct iscsi_log
373388
mutex_unlock(&sess->cmdsn_mutex);
374389

375390
return 0;
391+
392+
err:
393+
if (login->login_complete) {
394+
if (conn->rx_thread && conn->rx_thread_active) {
395+
send_sig(SIGINT, conn->rx_thread, 1);
396+
kthread_stop(conn->rx_thread);
397+
}
398+
if (conn->tx_thread && conn->tx_thread_active) {
399+
send_sig(SIGINT, conn->tx_thread, 1);
400+
kthread_stop(conn->tx_thread);
401+
}
402+
spin_lock(&iscsit_global->ts_bitmap_lock);
403+
bitmap_release_region(iscsit_global->ts_bitmap, conn->bitmap_id,
404+
get_order(1));
405+
spin_unlock(&iscsit_global->ts_bitmap_lock);
406+
}
407+
return -1;
376408
}
377409

378410
static void iscsi_target_sk_data_ready(struct sock *sk)

‎include/target/iscsi/iscsi_target_core.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,7 @@ struct iscsi_conn {
595595
int bitmap_id;
596596
int rx_thread_active;
597597
struct task_struct *rx_thread;
598+
struct completion rx_login_comp;
598599
int tx_thread_active;
599600
struct task_struct *tx_thread;
600601
/* list_head for session connection list */

0 commit comments

Comments
 (0)