From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa1.hgst.iphmx.com ([68.232.141.245]:38538 "EHLO esa1.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752467AbdEEPt1 (ORCPT ); Fri, 5 May 2017 11:49:27 -0400 From: Bart Van Assche To: "hch@lst.de" CC: "ddiss@suse.de" , "hare@suse.com" , "target-devel@vger.kernel.org" , "agrover@redhat.com" , "nab@linux-iscsi.org" , "stable@vger.kernel.org" Subject: Re: [PATCH 05/19] target: Allocate sg-list correctly Date: Fri, 5 May 2017 15:49:24 +0000 Message-ID: <1493999363.2744.3.camel@sandisk.com> References: <20170504225102.8931-1-bart.vanassche@sandisk.com> <20170504225102.8931-6-bart.vanassche@sandisk.com> <20170505090653.GA5248@lst.de> In-Reply-To: <20170505090653.GA5248@lst.de> Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-ID: <643ECEA00C0D3641B671D378CAA16462@namprd04.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: 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. >=20 > 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 ar= e: * When the iSCSI target driver receives a SCSI command it calls transport_init_se_cmd() to initialize struct se_cmd. The iSCSI target dri= ver 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 driv= ers to=A0transport_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 overflow= s 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 a= n sg-list per command that can store EDTL bytes instead of CDBL bytes. Bart.=