[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH] scsi: restrict buffer length to req->cmd.xfer
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-trivial] [PATCH] scsi: restrict buffer length to req->cmd.xfer for responses to INQUIRY commands. |
Date: |
Tue, 24 Jan 2012 14:53:03 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0 |
Am 23.01.2012 18:15, schrieb Thomas Higdon:
> This prevents the emulated SCSI device from trying to DMA more bytes to the
> initiator than are expected. Without this, the SCRIPTS code in the emulated
> LSI
> device eventually raises a DMA interrupt for a data overrun when an INQUIRY
> command whose buflen exceeds req->cmd.xfer is processed. It's the
> responsibility of the client to provide a request buffer and allocation
> length that are large enough for the result of the command.
>
> I'm no expert on SCSI (or qemu), but this appears to be more correct behavior
> than before, and makes my SCSI INQUIRY commands more successful.
>
> Before patch:
>
> # echo setverbose 2 > /proc/scsi/sym53c8xx/0
> # sginfo -s -TT /dev/sda
> cdb: 12 00 00 00 24 00
> inquiry response:
> 00 00 05 02 1f 00 00 12 51 45 4d 55 20 20 20 20
> 51 45 4d 55 20 48 41 52 44 44 49 53 4b 20 20 20
> 31 2e 30 2e
> cdb: 12 01 00 00 04 00
> [ 237.014083] sd 0:0:0:0: extraneous data discarded.
> [ 237.014437] sd 0:0:0:0: COMMAND FAILED (87 0 1).
> do_scsi_io: opcode=0x12: Host_status=0x07 [DID_ERROR]
> No serial number (error doing INQUIRY, supported VPDs)
>
> After:
>
> # echo setverbose 2 > /proc/scsi/sym53c8xx/0
> # sginfo -s -TT /dev/sda
> cdb: 12 00 00 00 24 00
> inquiry response:
> 00 00 05 02 1f 00 00 12 51 45 4d 55 20 20 20 20
> 51 45 4d 55 20 48 41 52 44 44 49 53 4b 20 20 20
> 31 2e 30 2e
> cdb: 12 01 00 00 04 00
> cdb: 12 01 80 00 04 00
> cdb: 12 01 80 00 06 00
> inquiry (evpd page (0x80) response:
> 00 80 00 02 32 37
> Serial Number '27'
>
> Command line:
> /usr/bin/qemu-system-x86_64 -S -M pc -enable-kvm -m 1024 -smp
> 1,sockets=1,cores=1,threads=1 -nodefconfig -nodefaults -chardev
> socket,id=charmonitor,path=/var/lib/libvirt/qemu/monitor,server,nowait -mon
> chardev=charmonitor,id=monitor,mode=readline -rtc base=utc -boot d -device
> lsi,id=scsi0,bus=pci.0,addr=0x5 -drive
> file=disk.0,if=none,id=drive-scsi0-0-0,format=qcow2 -device
> scsi-disk,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0,serial=27
> -drive
> file=disk.2,if=none,media=cdrom,id=drive-ide0-0-0,readonly=on,format=raw
> -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -usb
> -vnc 0.0.0.0:80 -vga cirrus
>
> Signed-off-by: Thomas Higdon <address@hidden>
> ---
> hw/scsi-disk.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 5d8bf53..71fe2a3 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -477,6 +477,9 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> "buffer size %zd\n", page_code, req->cmd.xfer);
> return -1;
> }
> + if (buflen > req->cmd.xfer) {
> + buflen = req->cmd.xfer;
> + }
> /* done with EVPD */
> return buflen;
> }
I wonder if it would make sense to make this check in a more central
place like scsi_send_command(). This way we would avoid similar bugs in
other commands.
Kevin