From patchwork Sat Aug 13 17:29:40 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: hw/scsi-bus.c: Fix use of uninitialised variable Date: Sat, 13 Aug 2011 07:29:40 -0000 From: Blue Swirl X-Patchwork-Id: 109942 Message-Id: To: Paolo Bonzini , Peter Maydell Cc: The OpenBIOS Mailinglist , qemu-devel@nongnu.org On Fri, Aug 12, 2011 at 7:22 PM, Blue Swirl wrote: > On Fri, Aug 12, 2011 at 4:49 PM, Peter Maydell wrote: >> Don't use req before it has been initialised in scsi_req_new(). >> This fixes a compile failure due to gcc complaining about this. > > It fixes a crash if the warning is ignored: > Configuration device id QEMU version 1 machine id 32 > > Program received signal SIGSEGV, Segmentation fault. > scsi_req_new (d=0x15e46b0, tag=0x0, lun=0x0, buf=0x7fffffffde41 "\022", >    hba_private=) at /src/qemu/hw/scsi-bus.c:375 > 375             if (req->cmd.lba != -1) { > (gdb) bt > #0  scsi_req_new (d=0x15e46b0, tag=0x0, lun=0x0, buf=0x7fffffffde41 "\022", >    hba_private=) at /src/qemu/hw/scsi-bus.c:375 > #1  0x000000000052c6ef in do_busid_cmd (s=0x15e2790, buf=0x0, >    busid=) at /src/qemu/hw/esp.c:247 > #2  0x000000000052cc5d in do_cmd (s=0x15e2790) at /src/qemu/hw/esp.c:270 > #3  handle_satn (s=0x15e2790) at /src/qemu/hw/esp.c:284 > #4  0x000000000052d174 in esp_mem_writeb (opaque=0x15e2790, >    addr=, val=0xc2) at /src/qemu/hw/esp.c:640 > #5  0x000000004003d1f5 in ?? () > #6  0x0000000001632330 in ?? () > #7  0x0000000001632280 in ?? () > #8  0x00007fffffffe180 in ?? () > #9  0x3d3d87e90d932400 in ?? () > #10 0x00007ffff7eefd00 in ?? () > #11 0x00000000004dc558 in tb_reset_jump_recursive2 (tb=0xffee100c) >    at /src/qemu/exec.c:1389 > #12 tb_reset_jump_recursive (tb=0xffee100c) at /src/qemu/exec.c:1395 > #13 0x000000000040bdea in qemu_notify_event () at /src/qemu/cpus.c:616 > #14 > #15 0x00000000004de681 in cpu_sparc_exec (env=0x1059600) >    at /src/qemu/cpu-exec.c:528 > #16 0x000000000040c1fc in tcg_cpu_exec () at /src/qemu/cpus.c:1064 > #17 cpu_exec_all () at /src/qemu/cpus.c:1105 > #18 0x0000000000519497 in main_loop (argc=, >    argv=, envp=) >    at /src/qemu/vl.c:1392 > #19 main (argc=, argv=, >    envp=) at /src/qemu/vl.c:3356 > (gdb) p req > $1 = > (gdb) p req->cmd > Cannot access memory at address 0x28 > (gdb) p req->cmd.lba > Cannot access memory at address 0x48 > >> Signed-off-by: Peter Maydell >> --- >>  hw/scsi-bus.c |    2 +- >>  1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c >> index f2af6cd..559d5a4 100644 >> --- a/hw/scsi-bus.c >> +++ b/hw/scsi-bus.c >> @@ -372,7 +372,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun, >>     } else { >>         trace_scsi_req_parsed(d->id, lun, tag, buf[0], >>                               cmd.mode, cmd.xfer); >> -        if (req->cmd.lba != -1) { >> +        if (cmd.lba != -1) { >>             trace_scsi_req_parsed_lba(d->id, lun, tag, buf[0], >>                                       cmd.lba); >>         } Something is very wrong with SCSI. OpenBIOS can't boot anymore: Configuration device id QEMU version 1 machine id 32 CPUs: 1 x FMI,MB86904 UUID: 00000000-0000-0000-0000-000000000000 Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:16 Type 'help' for detailed information Trying cdrom:d... Unhandled Exception 0x0000002a PC = 0xffd10bdc NPC = 0xffd10be0 Stopping execution This is due to division by zero in OpenBIOS drivers/esp.c. Bisecting reveals that this is due to c7b488721d6aafe32994ac63f8d690ae6d4729fa, SCSI devices now report Unit Attention status after reset. OpenBIOS does not handle this case and fails (block size is 0). First OpenBIOS issues Inquiry command, then if a device is present, Read Capacity. I tried adding Request Sense command after Inquiry, but then QEMU crashes: Configuration device id QEMU version 1 machine id 32 Initializing SCSI...dma1: Revision 2 ESP at 0xffdaa230, buffer va 0xffdab000 dva 0xfe000000 done Initializing SCSI devices...do_command: id 0, cmd[0] 0x80, status 0x80 do_command: id 1, cmd[0] 0x80, status 0x80 do_command: id 2, cmd[0] 0x80, status 0x91 do_command_reply: status 0x93 do_command: id 2, cmd[0] 0x80, status 0x93 request_sense id 2 failed request_sense id 2 sense key 128 do_command: id 2, cmd[0] 0x80, status 0x91 do_command_reply: status 0x93 read_capacity id 2 bs 2048 sectors 1295228 do_command: id 2, cmd[0] 0x80, status 0x91 *** glibc detected *** /src/qemu/obj-amd64/sparc-softmmu/qemu-system-sparc: double free or corruption (out): 0x00000000016407e0 *** } @@ -224,6 +227,26 @@ inquiry(esp_private_t *esp, sd_private_t *sd) } +static unsigned int +request_sense(esp_private_t *esp, sd_private_t *sd) +{ + /* Setup command = Request Sense */ + memset(esp->buffer, 0, 6); + esp->buffer[0] = 0x80; + esp->buffer[1] = REQUEST_SENSE; + + esp->buffer[5] = 252; + + if (do_command(esp, sd, 7, 252)) { + DPRINTF("request_sense id %d failed\n", sd->id); + DPRINTF("request_sense id %d sense key %d\n", sd->id, esp->buffer[0]); + return 0; + } + + DPRINTF("request_sense id %d sense key %d\n", sd->id, esp->buffer[0]); + return 1; +} + static void ob_sd_read_blocks(sd_private_t **sd) { @@ -478,7 +501,9 @@ ob_esp_init(unsigned int slot, uint64_t base, unsigned long espoffset, esp->sd[id].id = id; if (!inquiry(esp, &esp->sd[id])) continue; + request_sense(esp, &esp->sd[id]); read_capacity(esp, &esp->sd[id]); + request_sense(esp, &esp->sd[id]); #ifdef CONFIG_DEBUG_ESP dump_drive(&esp->sd[id]); diff --git a/drivers/esp.c b/drivers/esp.c index 78478f6..2766e87 100644 --- a/drivers/esp.c +++ b/drivers/esp.c @@ -28,6 +28,7 @@ #define BUFSIZE 4096 +#define CONFIG_DEBUG_ESP #ifdef CONFIG_DEBUG_ESP #define DPRINTF(fmt, args...) \ do { printk(fmt , ##args); } while (0) @@ -176,12 +177,14 @@ read_capacity(esp_private_t *esp, sd_private_t *sd) if (do_command(esp, sd, 11, 8)) { sd->sectors = 0; sd->bs = 0; - + DPRINTF("read_capacity id %d failed\n", sd->id); return 0; } sd->bs = (esp->buffer[4] << 24) | (esp->buffer[5] << 16) | (esp->buffer[6] << 8) | esp->buffer[7]; sd->sectors = ((esp->buffer[0] << 24) | (esp->buffer[1] << 16) | (esp->buffer[2] << 8) | esp->buffer[3]) * (sd->bs / 512); + DPRINTF("read_capacity id %d bs %d sectors %d\n", sd->id, sd->bs, + sd->sectors); return 1; } @@ -224,6 +227,26 @@ inquiry(esp_private_t *esp, sd_private_t *sd) } +static unsigned int +request_sense(esp_private_t *esp, sd_private_t *sd) +{ + /* Setup command = Request Sense */ + memset(esp->buffer, 0, 6); + esp->buffer[0] = 0x80; + esp->buffer[1] = REQUEST_SENSE; + + esp->buffer[5] = 252; + + if (do_command(esp, sd, 7, 252)) { + DPRINTF("request_sense id %d failed\n", sd->id); + DPRINTF("request_sense id %d sense key %d\n", sd->id, esp->buffer[0]); + return 0; + } + + DPRINTF("request_sense id %d sense key %d\n", sd->id, esp->buffer[0]); + return 1; +} + static void ob_sd_read_blocks(sd_private_t **sd) { @@ -478,7 +501,9 @@ ob_esp_init(unsigned int slot, uint64_t base, unsigned long espoffset, esp->sd[id].id = id; if (!inquiry(esp, &esp->sd[id])) continue; + request_sense(esp, &esp->sd[id]); read_capacity(esp, &esp->sd[id]); + request_sense(esp, &esp->sd[id]); #ifdef CONFIG_DEBUG_ESP dump_drive(&esp->sd[id]);