Skip to content

Commit 27bba9c

Browse files
committed
Merge tag 'scsi-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi
Pull SCSI fixes from James Bottomley: "Fixes for two fairly obscure but annoying when triggered races in iSCSI" * tag 'scsi-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi: scsi: target: iscsi: Fix cmd abort fabric stop race scsi: libiscsi: Fix NOP race condition
2 parents 4fd84bc + f361993 commit 27bba9c

3 files changed

Lines changed: 31 additions & 12 deletions

File tree

drivers/scsi/libiscsi.c

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -533,8 +533,8 @@ static void iscsi_complete_task(struct iscsi_task *task, int state)
533533
if (conn->task == task)
534534
conn->task = NULL;
535535

536-
if (conn->ping_task == task)
537-
conn->ping_task = NULL;
536+
if (READ_ONCE(conn->ping_task) == task)
537+
WRITE_ONCE(conn->ping_task, NULL);
538538

539539
/* release get from queueing */
540540
__iscsi_put_task(task);
@@ -738,6 +738,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
738738
task->conn->session->age);
739739
}
740740

741+
if (unlikely(READ_ONCE(conn->ping_task) == INVALID_SCSI_TASK))
742+
WRITE_ONCE(conn->ping_task, task);
743+
741744
if (!ihost->workq) {
742745
if (iscsi_prep_mgmt_task(conn, task))
743746
goto free_task;
@@ -941,8 +944,11 @@ static int iscsi_send_nopout(struct iscsi_conn *conn, struct iscsi_nopin *rhdr)
941944
struct iscsi_nopout hdr;
942945
struct iscsi_task *task;
943946

944-
if (!rhdr && conn->ping_task)
945-
return -EINVAL;
947+
if (!rhdr) {
948+
if (READ_ONCE(conn->ping_task))
949+
return -EINVAL;
950+
WRITE_ONCE(conn->ping_task, INVALID_SCSI_TASK);
951+
}
946952

947953
memset(&hdr, 0, sizeof(struct iscsi_nopout));
948954
hdr.opcode = ISCSI_OP_NOOP_OUT | ISCSI_OP_IMMEDIATE;
@@ -957,11 +963,12 @@ static int iscsi_send_nopout(struct iscsi_conn *conn, struct iscsi_nopin *rhdr)
957963

958964
task = __iscsi_conn_send_pdu(conn, (struct iscsi_hdr *)&hdr, NULL, 0);
959965
if (!task) {
966+
if (!rhdr)
967+
WRITE_ONCE(conn->ping_task, NULL);
960968
iscsi_conn_printk(KERN_ERR, conn, "Could not send nopout\n");
961969
return -EIO;
962970
} else if (!rhdr) {
963971
/* only track our nops */
964-
conn->ping_task = task;
965972
conn->last_ping = jiffies;
966973
}
967974

@@ -984,7 +991,7 @@ static int iscsi_nop_out_rsp(struct iscsi_task *task,
984991
struct iscsi_conn *conn = task->conn;
985992
int rc = 0;
986993

987-
if (conn->ping_task != task) {
994+
if (READ_ONCE(conn->ping_task) != task) {
988995
/*
989996
* If this is not in response to one of our
990997
* nops then it must be from userspace.
@@ -1923,7 +1930,7 @@ static void iscsi_start_tx(struct iscsi_conn *conn)
19231930
*/
19241931
static int iscsi_has_ping_timed_out(struct iscsi_conn *conn)
19251932
{
1926-
if (conn->ping_task &&
1933+
if (READ_ONCE(conn->ping_task) &&
19271934
time_before_eq(conn->last_recv + (conn->recv_timeout * HZ) +
19281935
(conn->ping_timeout * HZ), jiffies))
19291936
return 1;
@@ -2058,7 +2065,7 @@ enum blk_eh_timer_return iscsi_eh_cmd_timed_out(struct scsi_cmnd *sc)
20582065
* Checking the transport already or nop from a cmd timeout still
20592066
* running
20602067
*/
2061-
if (conn->ping_task) {
2068+
if (READ_ONCE(conn->ping_task)) {
20622069
task->have_checked_conn = true;
20632070
rc = BLK_EH_RESET_TIMER;
20642071
goto done;

drivers/target/iscsi/iscsi_target.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,7 @@ EXPORT_SYMBOL(iscsit_queue_rsp);
483483
void iscsit_aborted_task(struct iscsi_conn *conn, struct iscsi_cmd *cmd)
484484
{
485485
spin_lock_bh(&conn->cmd_lock);
486-
if (!list_empty(&cmd->i_conn_node) &&
487-
!(cmd->se_cmd.transport_state & CMD_T_FABRIC_STOP))
486+
if (!list_empty(&cmd->i_conn_node))
488487
list_del_init(&cmd->i_conn_node);
489488
spin_unlock_bh(&conn->cmd_lock);
490489

@@ -4083,12 +4082,22 @@ static void iscsit_release_commands_from_conn(struct iscsi_conn *conn)
40834082
spin_lock_bh(&conn->cmd_lock);
40844083
list_splice_init(&conn->conn_cmd_list, &tmp_list);
40854084

4086-
list_for_each_entry(cmd, &tmp_list, i_conn_node) {
4085+
list_for_each_entry_safe(cmd, cmd_tmp, &tmp_list, i_conn_node) {
40874086
struct se_cmd *se_cmd = &cmd->se_cmd;
40884087

40894088
if (se_cmd->se_tfo != NULL) {
40904089
spin_lock_irq(&se_cmd->t_state_lock);
4091-
se_cmd->transport_state |= CMD_T_FABRIC_STOP;
4090+
if (se_cmd->transport_state & CMD_T_ABORTED) {
4091+
/*
4092+
* LIO's abort path owns the cleanup for this,
4093+
* so put it back on the list and let
4094+
* aborted_task handle it.
4095+
*/
4096+
list_move_tail(&cmd->i_conn_node,
4097+
&conn->conn_cmd_list);
4098+
} else {
4099+
se_cmd->transport_state |= CMD_T_FABRIC_STOP;
4100+
}
40924101
spin_unlock_irq(&se_cmd->t_state_lock);
40934102
}
40944103
}

include/scsi/libiscsi.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ struct iscsi_task {
132132
void *dd_data; /* driver/transport data */
133133
};
134134

135+
/* invalid scsi_task pointer */
136+
#define INVALID_SCSI_TASK (struct iscsi_task *)-1l
137+
135138
static inline int iscsi_task_has_unsol_data(struct iscsi_task *task)
136139
{
137140
return task->unsol_r2t.data_length > task->unsol_r2t.sent;

0 commit comments

Comments
 (0)