From 20610794a23b438522ffdcea0e21dac5c67cdf61 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 7 Aug 2017 16:24:30 -0700 Subject: [PATCH] 4.9-stable patches added patches: iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch --- ...n-pdu-asynchronous-socket-close-oops.patch | 375 ++++++++++++++++++ queue-4.9/series | 1 + 2 files changed, 376 insertions(+) create mode 100644 queue-4.9/iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch diff --git a/queue-4.9/iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch b/queue-4.9/iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch new file mode 100644 index 00000000000..1004f0bf3e7 --- /dev/null +++ b/queue-4.9/iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch @@ -0,0 +1,375 @@ +From 25cdda95fda78d22d44157da15aa7ea34be3c804 Mon Sep 17 00:00:00 2001 +From: Nicholas Bellinger +Date: Wed, 24 May 2017 21:47:09 -0700 +Subject: iscsi-target: Fix initial login PDU asynchronous socket close OOPs + +From: Nicholas Bellinger + +commit 25cdda95fda78d22d44157da15aa7ea34be3c804 upstream. + +This patch fixes a OOPs originally introduced by: + + commit bb048357dad6d604520c91586334c9c230366a14 + Author: Nicholas Bellinger + Date: Thu Sep 5 14:54:04 2013 -0700 + + iscsi-target: Add sk->sk_state_change to cleanup after TCP failure + +which would trigger a NULL pointer dereference when a TCP connection +was closed asynchronously via iscsi_target_sk_state_change(), but only +when the initial PDU processing in iscsi_target_do_login() from iscsi_np +process context was blocked waiting for backend I/O to complete. + +To address this issue, this patch makes the following changes. + +First, it introduces some common helper functions used for checking +socket closing state, checking login_flags, and atomically checking +socket closing state + setting login_flags. + +Second, it introduces a LOGIN_FLAGS_INITIAL_PDU bit to know when a TCP +connection has dropped via iscsi_target_sk_state_change(), but the +initial PDU processing within iscsi_target_do_login() in iscsi_np +context is still running. For this case, it sets LOGIN_FLAGS_CLOSED, +but doesn't invoke schedule_delayed_work(). + +The original NULL pointer dereference case reported by MNC is now handled +by iscsi_target_do_login() doing a iscsi_target_sk_check_close() before +transitioning to FFP to determine when the socket has already closed, +or iscsi_target_start_negotiation() if the login needs to exchange +more PDUs (eg: iscsi_target_do_login returned 0) but the socket has +closed. For both of these cases, the cleanup up of remaining connection +resources will occur in iscsi_target_start_negotiation() from iscsi_np +process context once the failure is detected. + +Finally, to handle to case where iscsi_target_sk_state_change() is +called after the initial PDU procesing is complete, it now invokes +conn->login_work -> iscsi_target_do_login_rx() to perform cleanup once +existing iscsi_target_sk_check_close() checks detect connection failure. +For this case, the cleanup of remaining connection resources will occur +in iscsi_target_do_login_rx() from delayed workqueue process context +once the failure is detected. + +Reported-by: Mike Christie +Reviewed-by: Mike Christie +Tested-by: Mike Christie +Cc: Mike Christie +Reported-by: Hannes Reinecke +Cc: Hannes Reinecke +Cc: Sagi Grimberg +Cc: Varun Prakash +Signed-off-by: Nicholas Bellinger +Signed-off-by: Greg Kroah-Hartman + +--- + drivers/target/iscsi/iscsi_target_nego.c | 204 ++++++++++++++++++++----------- + include/target/iscsi/iscsi_target_core.h | 1 + 2 files changed, 138 insertions(+), 67 deletions(-) + +--- a/drivers/target/iscsi/iscsi_target_nego.c ++++ b/drivers/target/iscsi/iscsi_target_nego.c +@@ -490,14 +490,60 @@ static void iscsi_target_restore_sock_ca + + static int iscsi_target_do_login(struct iscsi_conn *, struct iscsi_login *); + +-static bool iscsi_target_sk_state_check(struct sock *sk) ++static bool __iscsi_target_sk_check_close(struct sock *sk) + { + if (sk->sk_state == TCP_CLOSE_WAIT || sk->sk_state == TCP_CLOSE) { +- pr_debug("iscsi_target_sk_state_check: TCP_CLOSE_WAIT|TCP_CLOSE," ++ pr_debug("__iscsi_target_sk_check_close: TCP_CLOSE_WAIT|TCP_CLOSE," + "returning FALSE\n"); +- return false; ++ return true; + } +- return true; ++ return false; ++} ++ ++static bool iscsi_target_sk_check_close(struct iscsi_conn *conn) ++{ ++ bool state = false; ++ ++ if (conn->sock) { ++ struct sock *sk = conn->sock->sk; ++ ++ read_lock_bh(&sk->sk_callback_lock); ++ state = (__iscsi_target_sk_check_close(sk) || ++ test_bit(LOGIN_FLAGS_CLOSED, &conn->login_flags)); ++ read_unlock_bh(&sk->sk_callback_lock); ++ } ++ return state; ++} ++ ++static bool iscsi_target_sk_check_flag(struct iscsi_conn *conn, unsigned int flag) ++{ ++ bool state = false; ++ ++ if (conn->sock) { ++ struct sock *sk = conn->sock->sk; ++ ++ read_lock_bh(&sk->sk_callback_lock); ++ state = test_bit(flag, &conn->login_flags); ++ read_unlock_bh(&sk->sk_callback_lock); ++ } ++ return state; ++} ++ ++static bool iscsi_target_sk_check_and_clear(struct iscsi_conn *conn, unsigned int flag) ++{ ++ bool state = false; ++ ++ if (conn->sock) { ++ struct sock *sk = conn->sock->sk; ++ ++ write_lock_bh(&sk->sk_callback_lock); ++ state = (__iscsi_target_sk_check_close(sk) || ++ test_bit(LOGIN_FLAGS_CLOSED, &conn->login_flags)); ++ if (!state) ++ clear_bit(flag, &conn->login_flags); ++ write_unlock_bh(&sk->sk_callback_lock); ++ } ++ return state; + } + + static void iscsi_target_login_drop(struct iscsi_conn *conn, struct iscsi_login *login) +@@ -537,6 +583,20 @@ static void iscsi_target_do_login_rx(str + + pr_debug("entering iscsi_target_do_login_rx, conn: %p, %s:%d\n", + conn, current->comm, current->pid); ++ /* ++ * If iscsi_target_do_login_rx() has been invoked by ->sk_data_ready() ++ * before initial PDU processing in iscsi_target_start_negotiation() ++ * has completed, go ahead and retry until it's cleared. ++ * ++ * Otherwise if the TCP connection drops while this is occuring, ++ * iscsi_target_start_negotiation() will detect the failure, call ++ * cancel_delayed_work_sync(&conn->login_work), and cleanup the ++ * remaining iscsi connection resources from iscsi_np process context. ++ */ ++ if (iscsi_target_sk_check_flag(conn, LOGIN_FLAGS_INITIAL_PDU)) { ++ schedule_delayed_work(&conn->login_work, msecs_to_jiffies(10)); ++ return; ++ } + + spin_lock(&tpg->tpg_state_lock); + state = (tpg->tpg_state == TPG_STATE_ACTIVE); +@@ -544,26 +604,12 @@ static void iscsi_target_do_login_rx(str + + if (!state) { + pr_debug("iscsi_target_do_login_rx: tpg_state != TPG_STATE_ACTIVE\n"); +- iscsi_target_restore_sock_callbacks(conn); +- iscsi_target_login_drop(conn, login); +- iscsit_deaccess_np(np, tpg, tpg_np); +- return; ++ goto err; + } + +- if (conn->sock) { +- struct sock *sk = conn->sock->sk; +- +- read_lock_bh(&sk->sk_callback_lock); +- state = iscsi_target_sk_state_check(sk); +- read_unlock_bh(&sk->sk_callback_lock); +- +- if (!state) { +- pr_debug("iscsi_target_do_login_rx, TCP state CLOSE\n"); +- iscsi_target_restore_sock_callbacks(conn); +- iscsi_target_login_drop(conn, login); +- iscsit_deaccess_np(np, tpg, tpg_np); +- return; +- } ++ if (iscsi_target_sk_check_close(conn)) { ++ pr_debug("iscsi_target_do_login_rx, TCP state CLOSE\n"); ++ goto err; + } + + conn->login_kworker = current; +@@ -581,34 +627,29 @@ static void iscsi_target_do_login_rx(str + flush_signals(current); + conn->login_kworker = NULL; + +- if (rc < 0) { +- iscsi_target_restore_sock_callbacks(conn); +- iscsi_target_login_drop(conn, login); +- iscsit_deaccess_np(np, tpg, tpg_np); +- return; +- } ++ if (rc < 0) ++ goto err; + + pr_debug("iscsi_target_do_login_rx after rx_login_io, %p, %s:%d\n", + conn, current->comm, current->pid); + + rc = iscsi_target_do_login(conn, login); + if (rc < 0) { +- iscsi_target_restore_sock_callbacks(conn); +- iscsi_target_login_drop(conn, login); +- iscsit_deaccess_np(np, tpg, tpg_np); ++ goto err; + } else if (!rc) { +- if (conn->sock) { +- struct sock *sk = conn->sock->sk; +- +- write_lock_bh(&sk->sk_callback_lock); +- clear_bit(LOGIN_FLAGS_READ_ACTIVE, &conn->login_flags); +- write_unlock_bh(&sk->sk_callback_lock); +- } ++ if (iscsi_target_sk_check_and_clear(conn, LOGIN_FLAGS_READ_ACTIVE)) ++ goto err; + } else if (rc == 1) { + iscsi_target_nego_release(conn); + iscsi_post_login_handler(np, conn, zero_tsih); + iscsit_deaccess_np(np, tpg, tpg_np); + } ++ return; ++ ++err: ++ iscsi_target_restore_sock_callbacks(conn); ++ iscsi_target_login_drop(conn, login); ++ iscsit_deaccess_np(np, tpg, tpg_np); + } + + static void iscsi_target_do_cleanup(struct work_struct *work) +@@ -656,31 +697,54 @@ static void iscsi_target_sk_state_change + orig_state_change(sk); + return; + } ++ state = __iscsi_target_sk_check_close(sk); ++ pr_debug("__iscsi_target_sk_close_change: state: %d\n", state); ++ + if (test_bit(LOGIN_FLAGS_READ_ACTIVE, &conn->login_flags)) { + pr_debug("Got LOGIN_FLAGS_READ_ACTIVE=1 sk_state_change" + " conn: %p\n", conn); ++ if (state) ++ set_bit(LOGIN_FLAGS_CLOSED, &conn->login_flags); + write_unlock_bh(&sk->sk_callback_lock); + orig_state_change(sk); + return; + } +- if (test_and_set_bit(LOGIN_FLAGS_CLOSED, &conn->login_flags)) { ++ if (test_bit(LOGIN_FLAGS_CLOSED, &conn->login_flags)) { + pr_debug("Got LOGIN_FLAGS_CLOSED=1 sk_state_change conn: %p\n", + conn); + write_unlock_bh(&sk->sk_callback_lock); + orig_state_change(sk); + return; + } ++ /* ++ * If the TCP connection has dropped, go ahead and set LOGIN_FLAGS_CLOSED, ++ * but only queue conn->login_work -> iscsi_target_do_login_rx() ++ * processing if LOGIN_FLAGS_INITIAL_PDU has already been cleared. ++ * ++ * When iscsi_target_do_login_rx() runs, iscsi_target_sk_check_close() ++ * will detect the dropped TCP connection from delayed workqueue context. ++ * ++ * If LOGIN_FLAGS_INITIAL_PDU is still set, which means the initial ++ * iscsi_target_start_negotiation() is running, iscsi_target_do_login() ++ * via iscsi_target_sk_check_close() or iscsi_target_start_negotiation() ++ * via iscsi_target_sk_check_and_clear() is responsible for detecting the ++ * dropped TCP connection in iscsi_np process context, and cleaning up ++ * the remaining iscsi connection resources. ++ */ ++ if (state) { ++ pr_debug("iscsi_target_sk_state_change got failed state\n"); ++ set_bit(LOGIN_FLAGS_CLOSED, &conn->login_flags); ++ state = test_bit(LOGIN_FLAGS_INITIAL_PDU, &conn->login_flags); ++ write_unlock_bh(&sk->sk_callback_lock); + +- state = iscsi_target_sk_state_check(sk); +- write_unlock_bh(&sk->sk_callback_lock); +- +- pr_debug("iscsi_target_sk_state_change: state: %d\n", state); ++ orig_state_change(sk); + +- if (!state) { +- pr_debug("iscsi_target_sk_state_change got failed state\n"); +- schedule_delayed_work(&conn->login_cleanup_work, 0); ++ if (!state) ++ schedule_delayed_work(&conn->login_work, 0); + return; + } ++ write_unlock_bh(&sk->sk_callback_lock); ++ + orig_state_change(sk); + } + +@@ -945,6 +1009,15 @@ static int iscsi_target_do_login(struct + if (iscsi_target_handle_csg_one(conn, login) < 0) + return -1; + if (login_rsp->flags & ISCSI_FLAG_LOGIN_TRANSIT) { ++ /* ++ * Check to make sure the TCP connection has not ++ * dropped asynchronously while session reinstatement ++ * was occuring in this kthread context, before ++ * transitioning to full feature phase operation. ++ */ ++ if (iscsi_target_sk_check_close(conn)) ++ return -1; ++ + login->tsih = conn->sess->tsih; + login->login_complete = 1; + iscsi_target_restore_sock_callbacks(conn); +@@ -971,21 +1044,6 @@ static int iscsi_target_do_login(struct + break; + } + +- if (conn->sock) { +- struct sock *sk = conn->sock->sk; +- bool state; +- +- read_lock_bh(&sk->sk_callback_lock); +- state = iscsi_target_sk_state_check(sk); +- read_unlock_bh(&sk->sk_callback_lock); +- +- if (!state) { +- pr_debug("iscsi_target_do_login() failed state for" +- " conn: %p\n", conn); +- return -1; +- } +- } +- + return 0; + } + +@@ -1252,13 +1310,25 @@ int iscsi_target_start_negotiation( + if (conn->sock) { + struct sock *sk = conn->sock->sk; + +- write_lock_bh(&sk->sk_callback_lock); +- set_bit(LOGIN_FLAGS_READY, &conn->login_flags); +- write_unlock_bh(&sk->sk_callback_lock); +- } ++ write_lock_bh(&sk->sk_callback_lock); ++ set_bit(LOGIN_FLAGS_READY, &conn->login_flags); ++ set_bit(LOGIN_FLAGS_INITIAL_PDU, &conn->login_flags); ++ write_unlock_bh(&sk->sk_callback_lock); ++ } ++ /* ++ * If iscsi_target_do_login returns zero to signal more PDU ++ * exchanges are required to complete the login, go ahead and ++ * clear LOGIN_FLAGS_INITIAL_PDU but only if the TCP connection ++ * is still active. ++ * ++ * Otherwise if TCP connection dropped asynchronously, go ahead ++ * and perform connection cleanup now. ++ */ ++ ret = iscsi_target_do_login(conn, login); ++ if (!ret && iscsi_target_sk_check_and_clear(conn, LOGIN_FLAGS_INITIAL_PDU)) ++ ret = -1; + +- ret = iscsi_target_do_login(conn, login); +- if (ret < 0) { ++ if (ret < 0) { + cancel_delayed_work_sync(&conn->login_work); + cancel_delayed_work_sync(&conn->login_cleanup_work); + iscsi_target_restore_sock_callbacks(conn); +--- a/include/target/iscsi/iscsi_target_core.h ++++ b/include/target/iscsi/iscsi_target_core.h +@@ -563,6 +563,7 @@ struct iscsi_conn { + #define LOGIN_FLAGS_READ_ACTIVE 1 + #define LOGIN_FLAGS_CLOSED 2 + #define LOGIN_FLAGS_READY 4 ++#define LOGIN_FLAGS_INITIAL_PDU 8 + unsigned long login_flags; + struct delayed_work login_work; + struct delayed_work login_cleanup_work; diff --git a/queue-4.9/series b/queue-4.9/series index 090ecb2dd99..ecf82bcc2d9 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -26,3 +26,4 @@ ext4-fix-overflow-caused-by-missing-cast-in-ext4_resize_fs.patch arm-dts-armada-38x-fix-irq-type-for-pca955.patch arm-dts-tango4-request-rgmii-rx-and-tx-clock-delays.patch media-platform-davinci-return-einval-for-vpfe_cmd_s_ccdc_raw_params-ioctl.patch +iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch -- 2.47.3