From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.hgst.iphmx.com ([216.71.154.42]:59577 "EHLO esa4.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751178AbdEHSHr (ORCPT ); Mon, 8 May 2017 14:07:47 -0400 From: Bart Van Assche To: "nab@linux-iscsi.org" CC: "hch@lst.de" , "ddiss@suse.de" , "hare@suse.com" , "target-devel@vger.kernel.org" , "agrover@redhat.com" , "stable@vger.kernel.org" Subject: Re: [PATCH 06/19] target: Fix data buffer size for VERIFY and WRITE AND VERIFY commands Date: Mon, 8 May 2017 18:07:43 +0000 Message-ID: <1494266862.2591.14.camel@sandisk.com> References: <20170504225102.8931-1-bart.vanassche@sandisk.com> <20170504225102.8931-7-bart.vanassche@sandisk.com> <1494197384.30469.34.camel@haakon3.risingtidesystems.com> In-Reply-To: <1494197384.30469.34.camel@haakon3.risingtidesystems.com> Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-ID: <26181E23A552924097B6605F436A79F3@namprd04.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: On Sun, 2017-05-07 at 15:49 -0700, Nicholas A. Bellinger wrote: > On Thu, 2017-05-04 at 15:50 -0700, Bart Van Assche wrote: > > For VERIFY and WRITE AND VERIFY commands the size of the SCSI > > Data-Out buffer can differ from the size of the data area on > > the storage medium that is affected by the command. Make sure > > that the Data-Out buffer size is computed correctly. Apparently > > this part got dropped from my previous VERIFY / WRITE AND VERIFY > > patch before I posted it due to rebasing. > >=20 > > Fixes: commit 0e2eb7d12eaa ("target: Fix VERIFY and WRITE VERIFY comman= d parsing") > > Signed-off-by: Bart Van Assche > > Cc: Hannes Reinecke > > Cc: Christoph Hellwig > > Cc: Andy Grover > > Cc: David Disseldorp > > Cc: > > --- > > drivers/target/target_core_sbc.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > >=20 > > diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_c= ore_sbc.c > > index a0ad618f1b1a..51489d96cb31 100644 > > --- a/drivers/target/target_core_sbc.c > > +++ b/drivers/target/target_core_sbc.c > > @@ -888,9 +888,10 @@ static sense_reason_t sbc_parse_verify(struct se_c= md *cmd, int *sectors, > > sense_reason_t > > sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops) > > { > > + enum { INVALID_SIZE =3D 1 }; > > struct se_device *dev =3D cmd->se_dev; > > unsigned char *cdb =3D cmd->t_task_cdb; > > - unsigned int size; > > + unsigned int size =3D INVALID_SIZE; > > u32 sectors =3D 0; > > sense_reason_t ret; > > =20 > > @@ -1212,7 +1213,7 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops = *ops) > > return TCM_ADDRESS_OUT_OF_RANGE; > > } > > =20 > > - if (!(cmd->se_cmd_flags & SCF_COMPARE_AND_WRITE)) > > + if (size =3D=3D INVALID_SIZE) > > size =3D sbc_get_size(cmd, sectors); > > } > > =20 >=20 > This patch has no effect. Hello Nic, That's a misinterpretation of your side. What this patch does is to ensure that 'size' remains zero if a VERIFY or WRITE AND VERIFY command is submitt= ed with BYTCHK =3D=3D 0. SBC mentions clearly that BYTCHK =3D=3D 0 means that = there is no Data-Out buffer, or in other words, that the size of the Data-Out buffer is zero. >>From the sg_verify man page: When --ndo=3DNDO is not given then the verify starts at the logical = block =A0=A0=A0=A0=A0=A0=A0address given by the --lba=3DLBA option and continues= =A0for=A0--count=3DCOUNT =A0=A0=A0=A0=A0=A0=A0blocks. In other words, sg_verify is able to submit a VERIFY command without Data-O= ut buffer (NDO is the size of the Data-Out buffer in bytes). > Anyways, I've fixed both cases and will post the proper fix inline > against patch #19. Do you perhaps mean patch "target: Fix sbc_parse_verify bytchk =3D 0 handli= ng"? (https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git/com= mit/?h=3Dfor-next&id=3Dd7d40280f868463d01a82186fd61cdc0e590381f) If you want to know my opinion about that patch: it doesn't fix anything bu= t breaks the sbc_parse_verify() function and definitely doesn't make the behavior of VERIFY nor WRITE AND VERIFY more compliant with the SCSI specs. Bart.=