Re: [PATCH] scsi: check inquiry buffer length to prevent crash

["Théo Maillart" <tmaillart@freebox.fr>]

[-- Attachment #1: Type: text/plain, Size: 3699 bytes --]

This crash appeared on the latest linux guests, most likely because of
this commit from the linux kernel:

v5.18-rc1-157-gc92a6b5d6335


On Wed, Apr 26, 2023 at 7:13 PM Théo Maillart <tmaillart@freebox.fr> wrote:

>
> Le mer. 26 avr. 2023 à 15:38, Théo Maillart <tmaillart@freebox.fr> a
> écrit :
>
>> Using linux 6.x guest, at boot time, an inquiry makes qemu crash.
>> Here is a trace of the scsi inquiry in question:
>>
>> scsi_req_parsed target 1 lun 0 tag 0x2cffb48 command 18 dir 1 length 4
>> scsi_req_parsed_lba target 1 lun 0 tag 0x2cffb48 command 18 lba 110592
>> scsi_req_alloc target 1 lun 0 tag 0x2cffb48
>> scsi_inquiry target 1 lun 0 tag 0x2cffb48 page 0x01/0xb0
>> scsi_generic_send_command Command: data= 0x12 0x01 0xb0 0x00 0x04 0x00
>> scsi_req_continue target 1 lun 0 tag 0x2cffb48
>> scsi_generic_read_data scsi_read_data tag=0x2cffb48
>> scsi_generic_aio_sgio_command generic aio sgio: tag=0x2cffb48 cmd=0x12
>> timeout=30000
>> scsi_generic_read_complete Data ready tag=0x2cffb48 len=4
>> scsi_req_data target 1 lun 0 tag 0x2cffb48 len 4
>> scsi_req_continue target 1 lun 0 tag 0x2cffb48
>> scsi_generic_read_data scsi_read_data tag=0x2cffb48
>> scsi_generic_command_complete_noio Command complete 0x7fb0870b80
>> tag=0x2cffb48 status=0
>> scsi_req_dequeue target 1 lun 0 tag 0x2cffb48
>>
>> And here is a backtrace from the crash:
>>
>>  #0  0x0000007face68580 in a_crash () at ./src/internal/atomic.h:250
>>  #1  get_nominal_size (end=0x7f6758f92c "", p=0x7f6758f920 "") at
>> src/malloc/mallocng/meta.h:168
>>  #2  __libc_free (p=0x7f6758f920) at src/malloc/mallocng/free.c:110
>>  #3  0x0000005592f93ed8 in scsi_free_request (req=0x7fac2c6b50) at
>> ../hw/scsi/scsi-generic.c:70
>>  #4  0x0000005592f869b8 in scsi_req_unref (req=0x7fac2c6b50) at
>> ../hw/scsi/scsi-bus.c:1382
>>  #5  0x0000005592f94b7c in scsi_read_complete (opaque=0x7fac2c6b50,
>> ret=0) at ../hw/scsi/scsi-generic.c:354
>>  #6  0x0000005593659b90 in blk_aio_complete (acb=0x7f66c206a0) at
>> ../block/block-backend.c:1527
>>  #7  0x000000559365a3c8 in blk_aio_ioctl_entry (opaque=0x7f66c206a0) at
>> ../block/block-backend.c:1735
>>  #8  0x00000055937dee64 in coroutine_bootstrap (self=0x7f672f77e0,
>> co=0x7f672f77e0) at ../util/coroutine-sigaltstack.c:104
>>  #9  0x00000055937deed8 in coroutine_trampoline (signal=12) at
>> ../util/coroutine-sigaltstack.c:145
>>  #10 <signal handler called>
>>  #11 __cp_end () at src/thread/aarch64/syscall_cp.s:30
>>  #12 0x0000007facea8214 in __syscall_cp_c (nr=133, u=<optimized out>,
>> v=<optimized out>, w=<optimized out>, x=<optimized out>,
>>      y=<optimized out>, z=<optimized out>) at
>> src/thread/pthread_cancel.c:33
>>  #13 0x0000007facefa020 in ?? ()
>>
>> Signed-off-by: Théo Maillart <tmaillart@freebox.fr>
>> ---
>>  hw/scsi/scsi-generic.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
>> index ac9fa662b4..25246589b7 100644
>> --- a/hw/scsi/scsi-generic.c
>> +++ b/hw/scsi/scsi-generic.c
>> @@ -191,7 +191,7 @@ static int scsi_handle_inquiry_reply(SCSIGenericReq
>> *r, SCSIDevice *s, int len)
>>      if ((s->type == TYPE_DISK || s->type == TYPE_ZBC) &&
>>          (r->req.cmd.buf[1] & 0x01)) {
>>          page = r->req.cmd.buf[2];
>> -        if (page == 0xb0) {
>> +        if (page == 0xb0 && r->buflen >= 12) {
>
>
> Actually the test should be r->buflen > 12
>
>
>>              uint64_t max_transfer = calculate_max_transfer(s);
>>              stl_be_p(&r->buf[8], max_transfer);
>>              /* Also take care of the opt xfer len. */
>> --
>> 2.40.0
>>
>>

[-- Attachment #2: Type: text/html, Size: 4918 bytes --]