On Sun, 2017-05-07 at 15:45 -0700, Nicholas A. Bellinger wrote:
> On Thu, 2017-05-04 at 15:50 -0700, Bart Van Assche wrote:
> > Avoid that the iSCSI target driver complains about "Initial page entry
> > out-of-bounds" and closes the connection if the SCSI Data-Out buffer
> > is larger than the buffer size specified through the CDB. This patch
> > prevents that running the libiscsi regression tests against LIO trigger
> > an infinite loop of libiscsi submitting a command, LIO closing the
> > connection and libiscsi resubmitting the same command.
> >
>
> This statement and patch is incorrect.
>
> target-core has always rejected SCF_SCSI_DATA_CDB WRITEs with
> overflow/underflow since day one:
>
> From target_cmd_size_check():
>
> } else if (size != cmd->data_length) {
> pr_warn("TARGET_CORE[%s]: Expected Transfer Length:"
> " %u does not match SCSI CDB Length: %u for SAM Opcode:"
> " 0x%02x\n", cmd->se_tfo->get_fabric_name(),
> cmd->data_length, size, cmd->t_task_cdb[0]);
>
> if (cmd->data_direction == DMA_TO_DEVICE &&
> cmd->se_cmd_flags & SCF_SCSI_DATA_CDB) {
> pr_err("Rejecting underflow/overflow WRITE data\n");
> return TCM_INVALID_CDB_FIELD;
> }
>
> ......
> }
Hello Nic,
That behavior is as far as I know not compliant with the SCSI specs. In e.g.
the libiscsi test suite there are many tests that verify that a SCSI target
implementation reports OVERFLOW or UNDERFLOW where LIO today reports INVALID
FIELD IN CDB. See e.g. https://github.com/sahlberg/libiscsi/blob/master/test-tool/test_read10_residuals.c.
>From RFC 3720 (apparently written from the point-of-view of transfers from
target to initiator):
bit 5 - (O) set for Residual Overflow. In this case, the Residual
Count indicates the number of bytes that were not transferred
because the initiator's Expected Data Transfer Length was not
sufficient.
bit 6 - (U) set for Residual Underflow. In this case, the Residual
Count indicates the number of bytes that were not transferred out
of the number of bytes that were expected to be transferred.
But even with the current implementation, why do you think that the reject by
target_cmd_size_check() would be sufficient to prevent the behavior I described?
>From source file iscsi_target.c:
static int
iscsit_handle_scsi_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd,
unsigned char *buf)
{
struct iscsi_scsi_req *hdr = (struct iscsi_scsi_req *)buf;
int rc, immed_data;
bool dump_payload = false;
rc = iscsit_setup_scsi_cmd(conn, cmd, buf);
if (rc < 0)
return 0;
/*
* Allocation iovecs needed for struct socket operations for
* traditional iSCSI block I/O.
*/
if (iscsit_allocate_iovecs(cmd) < 0) {
return iscsit_reject_cmd(cmd,
ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
}
immed_data = cmd->immediate_data;
rc = iscsit_process_scsi_cmd(conn, cmd, hdr);
if (rc < 0)
return rc;
else if (rc > 0)
dump_payload = true;
if (!immed_data)
return 0;
return iscsit_get_immediate_data(cmd, hdr, dump_payload);
}
In other words, if target_setup_cmd_from_cdb() returns a sense code (positive
value) iscsit_get_immediate_data() will call rx_data() to receive that immediate
data if there is immediate data and dump_payload == false. The code that controls
the value of dump_payload is as follows (from iscsit_process_scsi_cmd()):
if (cmd->sense_reason) {
if (cmd->reject_reason)
return 0;
return 1;
}
In other words, if both .sense_reason and .reject_reason are set before
iscsit_process_scsi_cmd() is called then iscsit_get_immediate_data() can call
rx_data() to receive more data than what fits in the allocated buffer.
Bart.