This fixes 2 bugs added in:
commit f36199355c64 ("scsi: target: iscsi: Fix cmd abort fabric stop
race")
If we have multiple sessions to the same se_device we can hit a race where
a LUN_RESET on one session cleans up the se_cmds from under another
session which is being closed. This results in the closing session freeing
its conn/session structs while they are still in use.
The bug is:
1. Session1 has IO se_cmd1.
2. Session2 can also have se_cmds for IO and optionally TMRs for ABORTS
but then gets a LUN_RESET.
3. The LUN_RESET on session2 sees the se_cmds on session1 and during
the drain stages marks them all with CMD_T_ABORTED.
4. session1 is now closed so iscsit_release_commands_from_conn only sees
se_cmds with the CMD_T_ABORTED bit set and returns immediately even
though we have outstanding commands.
5. session1's connection and session are freed.
6. The backend request for se_cmd1 completes and it accesses the freed
connection/session.
If session1 was executing only IO se_cmds and TAS is set on the se_cmd,
then we need to do a iscsit_free_cmd on those commands, so we wait on
their completion from LIO core and the backend.
If session1 was waiting on tmr se_cmds or TAS is not set then we need to
wait for those outstanding se_cmds to have their last put done so we
know no user is still accessing them when we free the session/conn.
This fixes the TAS set case, by adding a check so if we hit it we now call
iscsit_free_cmd. To handle the tmr se_cd and non TAS case, it hooks the
iscsit layer into the cmd counter code, so we can wait for all outstanding
commands before freeing the connection and possibly the session.
Fixes: f36199355c64 ("scsi: target: iscsi: Fix cmd abort fabric stop race")
Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
---
drivers/infiniband/ulp/isert/ib_isert.c | 13 +------------
drivers/target/iscsi/iscsi_target.c | 13 ++++++++++++-
drivers/target/target_core_transport.c | 6 ++++--
include/target/target_core_fabric.h | 2 ++
4 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index b360a1527cd1..600059d8a3a7 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -2501,17 +2501,6 @@ isert_wait4logout(struct isert_conn *isert_conn)
}
}
-static void
-isert_wait4cmds(struct iscsit_conn *conn)
-{
- isert_info("iscsit_conn %p\n", conn);
-
- if (conn->sess) {
- target_stop_session(conn->sess->se_sess);
- target_wait_for_sess_cmds(conn->sess->se_sess);
- }
-}
-
/**
* isert_put_unsol_pending_cmds() - Drop commands waiting for
* unsolicitate dataout
@@ -2559,7 +2548,7 @@ static void isert_wait_conn(struct iscsit_conn *conn)
ib_drain_qp(isert_conn->qp);
isert_put_unsol_pending_cmds(conn);
- isert_wait4cmds(conn);
+ target_wait_for_cmds(conn->cmd_cnt);
isert_wait4logout(isert_conn);
queue_work(isert_release_wq, &isert_conn->release_work);
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 7a8ffdf33bee..1c3470e4b50c 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4221,7 +4221,8 @@ static void iscsit_release_commands_from_conn(struct iscsit_conn *conn)
if (se_cmd->se_tfo != NULL) {
spin_lock_irq(&se_cmd->t_state_lock);
- if (se_cmd->transport_state & CMD_T_ABORTED) {
+ if (se_cmd->transport_state & CMD_T_ABORTED &&
+ !(se_cmd->transport_state & CMD_T_TAS)) {
/*
* LIO's abort path owns the cleanup for this,
* so put it back on the list and let
@@ -4244,6 +4245,14 @@ static void iscsit_release_commands_from_conn(struct iscsit_conn *conn)
iscsit_free_cmd(cmd, true);
}
+
+ /*
+ * Wait on commands that were cleaned up via the aborted_task path.
+ * LLDs that implement iscsit_wait_conn will already have waited for
+ * commands.
+ */
+ if (!conn->conn_transport->iscsit_wait_conn)
+ target_wait_for_cmds(conn->cmd_cnt);
}
static void iscsit_stop_timers_for_cmds(
@@ -4304,6 +4313,8 @@ int iscsit_close_connection(
iscsit_stop_nopin_response_timer(conn);
iscsit_stop_nopin_timer(conn);
+ target_stop_cmd_counter(conn->cmd_cnt);
+
if (conn->conn_transport->iscsit_wait_conn)
conn->conn_transport->iscsit_wait_conn(conn);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 90e3b1aef1f1..8bbf0c834b74 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -3174,13 +3174,14 @@ static void target_stop_cmd_counter_confirm(struct percpu_ref *ref)
* target_stop_cmd_counter - Stop new IO from being added to the counter.
* @cmd_cnt: counter to stop
*/
-static void target_stop_cmd_counter(struct target_cmd_counter *cmd_cnt)
+void target_stop_cmd_counter(struct target_cmd_counter *cmd_cnt)
{
pr_debug("Stopping command counter.\n");
if (!atomic_cmpxchg(&cmd_cnt->stopped, 0, 1))
percpu_ref_kill_and_confirm(&cmd_cnt->refcnt,
target_stop_cmd_counter_confirm);
}
+EXPORT_SYMBOL_GPL(target_stop_cmd_counter);
/**
* target_stop_session - Stop new IO from being queued on the session.
@@ -3196,7 +3197,7 @@ EXPORT_SYMBOL(target_stop_session);
* target_wait_for_cmds - Wait for outstanding cmds.
* @cmd_cnt: counter to wait for active I/O for.
*/
-static void target_wait_for_cmds(struct target_cmd_counter *cmd_cnt)
+void target_wait_for_cmds(struct target_cmd_counter *cmd_cnt)
{
int ret;
@@ -3212,6 +3213,7 @@ static void target_wait_for_cmds(struct target_cmd_counter *cmd_cnt)
wait_for_completion(&cmd_cnt->stop_done);
pr_debug("Waiting for cmds done.\n");
}
+EXPORT_SYMBOL_GPL(target_wait_for_cmds);
/**
* target_wait_for_sess_cmds - Wait for outstanding commands
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 4cbfb532a431..b188b1e90e1e 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -133,6 +133,8 @@ struct se_session *target_setup_session(struct se_portal_group *,
struct se_session *, void *));
void target_remove_session(struct se_session *);
+void target_stop_cmd_counter(struct target_cmd_counter *cmd_cnt);
+void target_wait_for_cmds(struct target_cmd_counter *cmd_cnt);
struct target_cmd_counter *target_alloc_cmd_counter(void);
void target_free_cmd_counter(struct target_cmd_counter *cmd_cnt);
--
2.25.1
