From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:57021)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1SpDSZ-00009z-CQ
	for qemu-devel@nongnu.org; Thu, 12 Jul 2012 03:05:17 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1SpDSR-0007MZ-LJ
	for qemu-devel@nongnu.org; Thu, 12 Jul 2012 03:05:11 -0400
Received: from mx1.redhat.com ([209.132.183.28]:21245)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1SpDSR-0007Ln-CJ
	for qemu-devel@nongnu.org; Thu, 12 Jul 2012 03:05:03 -0400
Message-ID: <4FFE771A.7030503@redhat.com>
Date: Thu, 12 Jul 2012 09:04:58 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1342075967-26378-1-git-send-email-ronniesahlberg@gmail.com>
	<1342075967-26378-2-git-send-email-ronniesahlberg@gmail.com>
In-Reply-To: <1342075967-26378-2-git-send-email-ronniesahlberg@gmail.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] SCSI: improve the lba-out-of-range tests
	for read/write/verify
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: qemu-devel@nongnu.org

Il 12/07/2012 08:52, Ronnie Sahlberg ha scritto:
> Improve the tests for the LBA to cover more cases, the new test looks like this
>         if (r->req.cmd.lba > r->req.cmd.lba + len
>         ||  r->req.cmd.lba + len > s->qdev.max_lba) {
> 
> For the 16 byte opcodes, the lba is a uint64, so the first check is to make sure that we do not wrap.
> For example if an opcode would specify the LBA:0xffffffffffffffff and LEN:2
> then lba+len would wrap to 1.
> 
> The second part of the test is to verify that ALL requested blocks are available, not just the first one.

Fixed code style for || and applied to scsi-next.

Thanks!

Paolo

> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  hw/scsi-disk.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index b2f3c0c..40c05de 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -1558,7 +1558,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
>          if (r->req.cmd.buf[1] & 0xe0) {
>              goto fail;
>          }
> -        if (r->req.cmd.lba > s->qdev.max_lba) {
> +        if (r->req.cmd.lba > r->req.cmd.lba + len
> +        ||  r->req.cmd.lba + len > s->qdev.max_lba) {
>              goto illegal_lba;
>          }
>          r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
> @@ -1581,7 +1582,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
>          if (r->req.cmd.buf[1] & 0xe0) {
>              goto fail;
>          }
> -        if (r->req.cmd.lba > s->qdev.max_lba) {
> +        if (r->req.cmd.lba > r->req.cmd.lba + len
> +        ||  r->req.cmd.lba + len > s->qdev.max_lba) {
>              goto illegal_lba;
>          }
>          r->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
>