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:
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;
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
