linux-kernel - [ 001/108] target: Fix ->data_length re-assignment bug with SCSI overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20121007225834.878055665@decadent.org.uk>
Date:	Sun, 07 Oct 2012 23:58:35 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc:	akpm@...ux-foundation.org, alan@...rguk.ukuu.org.uk,
	Nicholas Bellinger <nab@...ux-iscsi.org>,
	Roland Dreier <roland@...estorage.com>,
	Christoph Hellwig <hch@....de>,
	Boaz Harrosh <bharrosh@...asas.com>
Subject: [ 001/108] target: Fix ->data_length re-assignment bug with SCSI overflow

3.2-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Nicholas Bellinger <nab@...ux-iscsi.org>

commit 4c054ba63ad47ef244cfcfa1cea38134620a5bae upstream.

This patch fixes a long-standing bug with SCSI overflow handling
where se_cmd->data_length was incorrectly being re-assigned to
the larger CDB extracted allocation length, resulting in a number
of fabric level errors that would end up causing a session reset
in most cases.  So instead now:

 - Only re-assign se_cmd->data_length durining UNDERFLOW (to use the
   smaller value)
 - Use existing se_cmd->data_length for OVERFLOW (to use the smaller
   value)

This fix has been tested with the following CDB to generate an
SCSI overflow:

  sg_raw -r512 /dev/sdc 28 0 0 0 0 0 0 0 9 0

Tested using iscsi-target, tcm_qla2xxx, loopback and tcm_vhost fabric
ports.  Here is a bit more detail on each case:

 - iscsi-target: Bug with open-iscsi with overflow, sg_raw returns
                 -3584 bytes of data.
 - tcm_qla2xxx: Working as expected, returnins 512 bytes of data
 - loopback: sg_raw returns CHECK_CONDITION, from overflow rejection
             in transport_generic_map_mem_to_cmd()
 - tcm_vhost: Same as loopback

Reported-by: Roland Dreier <roland@...estorage.com>
Cc: Roland Dreier <roland@...estorage.com>
Cc: Christoph Hellwig <hch@....de>
Cc: Boaz Harrosh <bharrosh@...asas.com>
Signed-off-by: Nicholas Bellinger <nab@...ux-iscsi.org>
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 drivers/target/target_core_transport.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -3039,15 +3039,20 @@ static int transport_generic_cmd_sequenc
 			/* Returns CHECK_CONDITION + INVALID_CDB_FIELD */
 			goto out_invalid_cdb_field;
 		}
-
+		/*
+		 * For the overflow case keep the existing fabric provided
+		 * ->data_length.  Otherwise for the underflow case, reset
+		 * ->data_length to the smaller SCSI expected data transfer
+		 * length.
+		 */
 		if (size > cmd->data_length) {
 			cmd->se_cmd_flags |= SCF_OVERFLOW_BIT;
 			cmd->residual_count = (size - cmd->data_length);
 		} else {
 			cmd->se_cmd_flags |= SCF_UNDERFLOW_BIT;
 			cmd->residual_count = (cmd->data_length - size);
+			cmd->data_length = size;
 		}
-		cmd->data_length = size;
 	}
 
 	/* reject any command that we don't have a handler for */


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/