[PATCH-for-stable 0/5] target: Recently merged fixes for stable

10 views
Skip to first unread message

Nicholas A. Bellinger

unread,
Sep 4, 2013, 12:42:17 AM9/4/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

Hi Greg,

This set of target fixes was just merged into mainline below, and would
have been merged before v3.11 was released but unfortuately my PULL
request(s) ended up in Linus's spam folder. :-(

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dcaaaeac871ff73043c616db3b2f91482637801d

Two of the patches where not marked as stable in target-pending.git, so
I'm sending the entire series out preemptively now to let you know that:

1) They all should be marked for stable, and
2) should be included immediately, given the already extended delay.

Patch #1 is marked for v3.0+. (it will likely fail to apply at a certain
point, but I'll fix that up later)

Patch #2 -> #4 are marked for v3.10+, and Patch #5 is marked for v3.11+

Please let me know if you have any questions.

Thank you,

--nab

Nicholas Bellinger (5):
target: Fix trailing ASCII space usage in INQUIRY vendor+model
iscsi-target: Fix ImmediateData=Yes failure regression in >= v3.10
iscsi-target: Fix iscsit_transport reference leak during NP thread
reset
iscsi-target: Fix potential NULL pointer in solicited NOPOUT reject
target: Fix se_cmd->state_list leak regression during WRITE failure

drivers/target/iscsi/iscsi_target.c | 17 ++++++++++-------
drivers/target/iscsi/iscsi_target_login.c | 9 ++++-----
drivers/target/target_core_spc.c | 9 ++++++---
drivers/target/target_core_transport.c | 11 +++++++++++
4 files changed, 31 insertions(+), 15 deletions(-)

--
1.7.2.5

Nicholas A. Bellinger

unread,
Sep 4, 2013, 12:44:24 AM9/4/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger

Nicholas A. Bellinger

unread,
Sep 4, 2013, 12:44:25 AM9/4/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

commit ee60bddba5a5f23e39598195d944aa0eb2d455e5 upstream.

This patch fixes spc_emulate_inquiry_std() to add trailing ASCII
spaces for INQUIRY vendor + model fields following SPC-4 text:

"ASCII data fields described as being left-aligned shall have any
unused bytes at the end of the field (i.e., highest offset) and
the unused bytes shall be filled with ASCII space characters (20h)."

This addresses a problem with Falconstor NSS multipathing.

Reported-by: Tomas Molota <tomas....@lightstorm.sk>
Cc: <sta...@vger.kernel.org> # 3.0+
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_spc.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 4cb667d..9fabbf7 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -97,9 +97,12 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)

buf[7] = 0x2; /* CmdQue=1 */

- snprintf(&buf[8], 8, "LIO-ORG");
- snprintf(&buf[16], 16, "%s", dev->t10_wwn.model);
- snprintf(&buf[32], 4, "%s", dev->t10_wwn.revision);
+ memcpy(&buf[8], "LIO-ORG ", 8);
+ memset(&buf[16], 0x20, 16);
+ memcpy(&buf[16], dev->t10_wwn.model,
+ min_t(size_t, strlen(dev->t10_wwn.model), 16));
+ memcpy(&buf[32], dev->t10_wwn.revision,
+ min_t(size_t, strlen(dev->t10_wwn.revision), 4));
buf[4] = 31; /* Set additional length to 31 */

return 0;
--
1.7.2.5

Nicholas A. Bellinger

unread,
Sep 4, 2013, 12:44:26 AM9/4/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

commit 9d86a2befceb06ee83c1a588915e6d6e0abef797 upstream.

This patch addresses a regression bug within ImmediateData=Yes failure
handling that ends up triggering an OOPs within >= v3.10 iscsi-target
code.

The problem occurs when iscsit_process_scsi_cmd() does the call to
target_put_sess_cmd(), and once again in iscsit_get_immediate_data()
that is triggered during two different cases:

- When iscsit_sequence_cmd() returns CMDSN_LOWER_THAN_EXP, for which
the descriptor state will already have been set to ISTATE_REMOVE
by iscsit_sequence_cmd(), and
- When iscsi_cmd->sense_reason is set, for which iscsit_execute_cmd()
will have already called transport_send_check_condition_and_sense()
to queue the exception response.

It changes iscsit_process_scsi_cmd() to drop the early call, and makes
iscsit_get_immediate_data() call target_put_sess_cmd() from a single
location after dumping the immediate data for the failed command.

The regression was initially introduced in commit:

commit 561bf15892375597ee59d473a704a3e634c4f311
Author: Nicholas Bellinger <n...@linux-iscsi.org>
Date: Wed Jul 3 03:58:58 2013 -0700

iscsi-target: Fix iscsit_sequence_cmd reject handling for iser

Cc: <sta...@vger.kernel.org> # 3.10+
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/iscsi/iscsi_target.c | 9 ++-------
1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index f73da43..8fd359c 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1086,7 +1086,6 @@ int iscsit_process_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
if (cmd->reject_reason)
return 0;

- target_put_sess_cmd(conn->sess->se_sess, &cmd->se_cmd);
return 1;
}
/*
@@ -1124,14 +1123,10 @@ after_immediate_data:
*/
cmdsn_ret = iscsit_sequence_cmd(cmd->conn, cmd,
(unsigned char *)hdr, hdr->cmdsn);
- if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER) {
+ if (cmdsn_ret == CMDSN_ERROR_CANNOT_RECOVER)
return -1;
- } else if (cmdsn_ret == CMDSN_LOWER_THAN_EXP) {
- target_put_sess_cmd(conn->sess->se_sess, &cmd->se_cmd);
- return 0;
- }

- if (cmd->sense_reason) {
+ if (cmd->sense_reason || cmdsn_ret == CMDSN_LOWER_THAN_EXP) {
int rc;

rc = iscsit_dump_data_payload(cmd->conn,
--
1.7.2.5

Nicholas A. Bellinger

unread,
Sep 4, 2013, 12:44:27 AM9/4/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

commit c9a03c12464c851e691e8d5b6c9deba779c512e0 upstream.

This patch fixes a bug in __iscsi_target_login_thread() where an explicit
network portal thread reset ends up leaking the iscsit_transport module
reference, along with the associated iscsi_conn allocation.

This manifests itself with iser-target where a NP reset causes the extra
iscsit_transport reference to be taken in iscsit_conn_set_transport()
during the reset, which prevents the ib_isert module from being unloaded
after the NP thread shutdown has finished.

Cc: <sta...@vger.kernel.org> # v3.10+
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/iscsi/iscsi_target_login.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 3402241..bc788c5 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -1163,12 +1163,11 @@ static int __iscsi_target_login_thread(struct iscsi_np *np)
if (np->np_thread_state == ISCSI_NP_THREAD_RESET) {
spin_unlock_bh(&np->np_thread_lock);
complete(&np->np_restart_comp);
- if (ret == -ENODEV) {
- iscsit_put_transport(conn->conn_transport);
- kfree(conn);
- conn = NULL;
+ iscsit_put_transport(conn->conn_transport);
+ kfree(conn);
+ conn = NULL;
+ if (ret == -ENODEV)
goto out;
- }
/* Get another socket */
return 1;
}
--
1.7.2.5

Nicholas A. Bellinger

unread,
Sep 4, 2013, 12:44:28 AM9/4/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

commit 28aaa950320fc7b8df3f6d2d34fa7833391a9b72 upstream.

This patch addresses a potential NULL pointer dereference regression in
iscsit_setup_nop_out() code, specifically for two cases when a solicited
NOPOUT triggers a ISCSI_REASON_PROTOCOL_ERROR reject to be generated.

This is because iscsi_cmd is expected to be NULL for solicited NOPOUT
case before iscsit_process_nop_out() locates the descriptor via TTT
using iscsit_find_cmd_from_ttt().

This regression was originally introduced in:

commit ba159914086f06532079fc15141f46ffe7e04a41
Author: Nicholas Bellinger <n...@linux-iscsi.org>
Date: Wed Jul 3 03:48:24 2013 -0700

iscsi-target: Fix iscsit_add_reject* usage for iser

Cc: <sta...@vger.kernel.org> # 3.10+
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/iscsi/iscsi_target.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 8fd359c..3a17930 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1522,6 +1522,10 @@ int iscsit_setup_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
if (hdr->itt == RESERVED_ITT && !(hdr->opcode & ISCSI_OP_IMMEDIATE)) {
pr_err("NOPOUT ITT is reserved, but Immediate Bit is"
" not set, protocol error.\n");
+ if (!cmd)
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ (unsigned char *)hdr);
+
return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR,
(unsigned char *)hdr);
}
@@ -1531,6 +1535,10 @@ int iscsit_setup_nop_out(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
" greater than MaxXmitDataSegmentLength: %u, protocol"
" error.\n", payload_length,
conn->conn_ops->MaxXmitDataSegmentLength);
+ if (!cmd)
+ return iscsit_add_reject(conn, ISCSI_REASON_PROTOCOL_ERROR,
+ (unsigned char *)hdr);
+
return iscsit_reject_cmd(cmd, ISCSI_REASON_PROTOCOL_ERROR,
(unsigned char *)hdr);
}
--
1.7.2.5

Nicholas A. Bellinger

unread,
Sep 4, 2013, 12:44:29 AM9/4/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

commit c130480b129fbfd7932ad7af3f4ffcea630b027f upstream.

This patch addresses a v3.11 specific regression where se_cmd->state_list
was being leaked during a fabric WRITE failure, when the fabric releases
an associated se_cmd descriptor before I/O submission occurs, and normal
fast path callbacks have a chance to call target_remove_from_state_list().

It was manifesting with Poison overwritten messages with iscsi-target
once an ImmediateData payload CRC32C failure occured.

This bug was originally introduced during v3.11-rc1 with the following
commit:

commit 0b66818ac6de67a6125ae203272fb76e79b3a20f
Author: Nicholas Bellinger <n...@linux-iscsi.org>
Date: Thu Jun 6 01:36:41 2013 -0700

target: Drop unnecessary CMD_T_DEV_ACTIVE check from transport_lun_remove_cmd

Cc: <sta...@vger.kernel.org> # 3.11+
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_transport.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 7172d00..d8e49d7 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2134,6 +2134,7 @@ static void transport_write_pending_qf(struct se_cmd *cmd)

int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks)
{
+ unsigned long flags;
int ret = 0;

if (!(cmd->se_cmd_flags & SCF_SE_LUN_CMD)) {
@@ -2144,6 +2145,16 @@ int transport_generic_free_cmd(struct se_cmd *cmd, int wait_for_tasks)
} else {
if (wait_for_tasks)
transport_wait_for_tasks(cmd);
+ /*
+ * Handle WRITE failure case where transport_generic_new_cmd()
+ * has already added se_cmd to state_list, but fabric has
+ * failed command before I/O submission.
+ */
+ if (cmd->state_active) {
+ spin_lock_irqsave(&cmd->t_state_lock, flags);
+ target_remove_from_state_list(cmd);
+ spin_unlock_irqrestore(&cmd->t_state_lock, flags);
+ }

if (cmd->se_lun)
transport_lun_remove_cmd(cmd);
--
1.7.2.5

Nicholas A. Bellinger

unread,
Sep 5, 2013, 12:57:55 AM9/5/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

commit ee60bddba5a5f23e39598195d944aa0eb2d455e5 upstream.

This patch fixes spc_emulate_inquiry_std() to add trailing ASCII
spaces for INQUIRY vendor + model fields following SPC-4 text:

"ASCII data fields described as being left-aligned shall have any
unused bytes at the end of the field (i.e., highest offset) and
the unused bytes shall be filled with ASCII space characters (20h)."

This addresses a problem with Falconstor NSS multipathing.

Reported-by: Tomas Molota <tomas....@lightstorm.sk>
Cc: <sta...@vger.kernel.org> # 3.4.y
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_cdb.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c
index 52a5f62..2d88ce8 100644
--- a/drivers/target/target_core_cdb.c
+++ b/drivers/target/target_core_cdb.c
@@ -97,9 +97,12 @@ target_emulate_inquiry_std(struct se_cmd *cmd, char *buf)

buf[7] = 0x2; /* CmdQue=1 */

- snprintf(&buf[8], 8, "LIO-ORG");
- snprintf(&buf[16], 16, "%s", dev->se_sub_dev->t10_wwn.model);
- snprintf(&buf[32], 4, "%s", dev->se_sub_dev->t10_wwn.revision);
+ memcpy(&buf[8], "LIO-ORG ", 8);
+ memset(&buf[16], 0x20, 16);
+ memcpy(&buf[16], dev->se_sub_dev->t10_wwn.model,
+ min_t(size_t, strlen(dev->se_sub_dev->t10_wwn.model), 16));
+ memcpy(&buf[32], dev->se_sub_dev->t10_wwn.revision,
+ min_t(size_t, strlen(dev->se_sub_dev->t10_wwn.revision), 4));

Nicholas A. Bellinger

unread,
Sep 5, 2013, 12:58:53 AM9/5/13
to target-devel, Greg-KH, Stable, Nicholas Bellinger
From: Nicholas Bellinger <n...@linux-iscsi.org>

commit ee60bddba5a5f23e39598195d944aa0eb2d455e5 upstream.

This patch fixes spc_emulate_inquiry_std() to add trailing ASCII
spaces for INQUIRY vendor + model fields following SPC-4 text:

"ASCII data fields described as being left-aligned shall have any
unused bytes at the end of the field (i.e., highest offset) and
the unused bytes shall be filled with ASCII space characters (20h)."

This addresses a problem with Falconstor NSS multipathing.

Reported-by: Tomas Molota <tomas....@lightstorm.sk>
Cc: <sta...@vger.kernel.org> # 3.0.y
Signed-off-by: Nicholas Bellinger <n...@linux-iscsi.org>
---
drivers/target/target_core_cdb.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_cdb.c b/drivers/target/target_core_cdb.c
index 0558401..f8c49ab 100644
--- a/drivers/target/target_core_cdb.c
+++ b/drivers/target/target_core_cdb.c
@@ -117,11 +117,12 @@ target_emulate_inquiry_std(struct se_cmd *cmd)
return 0;
}

- snprintf((unsigned char *)&buf[8], 8, "LIO-ORG");
- snprintf((unsigned char *)&buf[16], 16, "%s",
- &DEV_T10_WWN(dev)->model[0]);
- snprintf((unsigned char *)&buf[32], 4, "%s",
- &DEV_T10_WWN(dev)->revision[0]);
Reply all
Reply to author
Forward
0 new messages