Re: [PATCH 05/19] target: Allocate sg-list correctly

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

On Fri, 2017-05-05 at 11:06 +0200, Christoph Hellwig wrote:
> On Thu, May 04, 2017 at 03:50:48PM -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.
> 
> Can you add a bit more of an explanation of why this happens?  I've
> just tried to verify the area, but at least while sitting in a conference
> talk I can't quite make sense of the changes.

Hello Christoph,

The aspects of SCSI command processing that are relevant in this context are:
* When the iSCSI target driver receives a SCSI command it calls
  transport_init_se_cmd() to initialize struct se_cmd. The iSCSI target driver
  passes the number of bytes that will be transferred into the "data_length"
  argument of transport_init_se_cmd(). That function stores the data length in
  the .data_length member of struct se_cmd. The value passed by target drivers
  to transport_init_se_cmd() is what is called the Expected Data Transfer
  Length (EDTL) in the iSCSI RFC.
* After CDB parsing has finished target_cmd_size_check() is called. If EDTL
  exceeds the data buffer size extracted from the SCSI CDB (CDBL) then
  .data_length is reduced to CDBL.
* Next target_alloc_sgl() allocates an sg-list for .data_length bytes (CDBL).
* iscsit_allocate_iovecs() allocates a struct kvec (.iov_data) also for
  .data_length bytes (CDBL).
* iscsit_get_dataout() calls rx_data() and tries to store EDTL bytes in the
  allocated struct kvec. If EDTL > CDBL then .iov_data overflows and this
  usually triggers a crash. With the patch that prevents .iov_data overflows
  the initiator is disconnected. In the case of libiscsi, it keeps retrying
  forever to resubmit SCSI commands for which EDTL > CDBL.

In other words, initially EDTL is stored into .data_length and later on the
value of .data_length changes to CDBL. My proposal to avoid the buffer
overflows is to store both EDTL and CDBL in struct se_cmd and to allocate an
sg-list per command that can store EDTL bytes instead of CDBL bytes.

Bart.