diff mbox

[06/19] target: Fix data buffer size for VERIFY and WRITE AND VERIFY commands

Message ID 1494479368.16894.127.camel@haakon3.risingtidesystems.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nicholas A. Bellinger May 11, 2017, 5:09 a.m. UTC 
On Wed, 2017-05-10 at 15:16 +0000, Bart Van Assche wrote:
> On Tue, 2017-05-09 at 21:28 -0700, Nicholas A. Bellinger wrote:
> > Attempting to make 'size' in sbc_parse_cdb() enforce what the spec says,
> > instead of what it actually is in the received CDB is completely wrong.
> 
> Please look again at e.g. the sg_verify source code. If it sets BYTCHK to
> zero it doesn't submit a Data-Out buffer. The only initiator that submits
> a Data-Out buffer and sets BYTCHK to zero is libiscsi when testing overflow
> behavior.

To repeat, it doesn't matter what the spec says, or what some host sends
or doesn't send.

The usage of 'size' in sbc_parse_cdb() is strictly for the value of the
transfer length extracted from a received CDB, and used to determine
overflow or underflow vs. fabric EDTL within target_cmd_size_check().

Any patch that attempts to arbitrarily set this to a value other than
what was received by a CDB that contains a transfer length field is
wrong.

As mentioned earlier, if you're genuinely concerned about btychk = 0
with a non zero transfer length, then I'm happy to apply the following
patch post -rc1:


--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 2cc8753..af618a6 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -869,6 +869,9 @@  static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, unsigned int *sectors
 
        switch (bytchk) {
        case 0:
+               if (*sectors)
+                       return TCM_INVALID_CDB_FIELD;
+               /* fall-through */
        case 1:
                cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
                break;