Patch "iscsi-target: Fix initial login PDU asynchronous socket close OOPs" has been added to the 4.4-stable tree
Subsystems (2)
This is a note to let you know that I've just added the patch titled
iscsi-target: Fix initial login PDU asynchronous socket close OOPs
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From
25cdda95fda78d22d44157da15aa7ea34be3c804Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger <[email protected]>
Date: Wed, 24 May 2017 21:47:09 -0700
Subject: iscsi-target: Fix initial login PDU asynchronous socket close OOPs
From: Nicholas Bellinger <[email protected]>
commit 25cdda95fda78d22d44157da15aa7ea34be3c804 upstream.
This patch fixes a OOPs originally introduced by:
commit bb048357dad6d604520c91586334c9c230366a14
Author: Nicholas Bellinger <[email protected]>
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.
--- drivers/target/iscsi/iscsi_target_nego.c | 204 ++++++++++++++++++++----------- include/target/iscsi/iscsi_target_core.h | 1 2 files changed, 138 insertions(+), 67 deletions(-)
drivers/target/iscsi/iscsi_target_nego.c +137 -67
@@ -489,14 +489,60 @@ static void iscsi_target_restore_sock_castatic 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)
@@ -536,6 +582,20 @@ static void iscsi_target_do_login_rx(strpr_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 );
@@ -543,26 +603,12 @@ static void iscsi_target_do_login_rx(strif (!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;
@@ -580,34 +626,29 @@ static void iscsi_target_do_login_rx(strflush_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)
@@ -655,31 +696,54 @@ static void iscsi_target_sk_state_changeorig_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); }
@@ -944,6 +1008,15 @@ static int iscsi_target_do_login(structif (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);
@@ -970,21 +1043,6 @@ static int iscsi_target_do_login(structbreak ; } -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 ; }
@@ -1251,13 +1309,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);
include/target/iscsi/iscsi_target_core.h +1 -0
@@ -562,6 +562,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 8unsigned long login_flags;struct delayed_work login_work;struct delayed_work login_cleanup_work;
Patches currently in stable-queue which might be from [email protected] are
queue-4.4/iscsi-target-fix-delayed-logout-processing-greater-than-seconds_for_logout_comp.patch queue-4.4/iscsi-target-always-wait-for-kthread_should_stop-before-kthread-exit.patch queue-4.4/iscsi-target-fix-early-sk_data_ready-login_flags_ready-race.patch queue-4.4/iscsi-target-fix-initial-login-pdu-asynchronous-socket-close-oops.patch queue-4.4/iser-target-avoid-isert_conn-cm_id-dereference-in-isert_login_recv_done.patch queue-4.4/target-avoid-mappedlun-symlink-creation-during-lun-shutdown.patch