Message ID | 1381503951-27985-55-git-send-email-kwolf@redhat.com |
---|---|
State | New |
Headers | show |
Am 11.10.2013 17:05, schrieb Kevin Wolf: > IF_NONE allows read-only, which makes forbidding it in this place > for other types pretty much pointless. > > Instead, make sure that all devices for which the check would have > errored out check in their init function that they don't get a read-only > BlockDriverState. This catches even cases where IF_NONE and -device is > used. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > This patch breaks current QEMU (SIGSEGV with ARM in several test scenarios): $ git bisect bad 4f8a066b5fc254eeaabbbde56ba4f5b29cc68fdf is the first bad commit commit 4f8a066b5fc254eeaabbbde56ba4f5b29cc68fdf Author: Kevin Wolf <kwolf@redhat.com> Date: Fri Sep 13 15:51:47 2013 +0200 blockdev: Remove IF_* check for read-only blockdev_init [...] See the gdb protocol below for more details (Linux x86_64 host, default configuration). I got a bug report from a Windows user, but the crash is not OS specific. Regards, Stefan (gdb) r Starting program: bin/arm-softmmu/qemu-system-arm -M versatilepb -L pc-bios -kernel vmlinuz-2.6.32-5-versatile -initrd initrd.img-2.6.32-5-versatile -sd debian_squeeze_armel_standard.qcow2 -append root=/dev/sda1 [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Breakpoint 5, pl181_init (sbd=0x5555565d3020) at hw/sd/pl181.c:482 482 { (gdb) i s #0 pl181_init (sbd=0x5555565d3020) at hw/sd/pl181.c:482 #1 0x00005555556d10e8 in sysbus_device_init (dev=0x5555565d3020) at hw/core/sysbus.c:143 #2 0x00005555556ce6d3 in device_realize (dev=0x5555565d3020, err=0x7fffffffdb08) at hw/core/qdev.c:178 #3 0x00005555556d002a in device_set_realized (obj=0x5555565d3020, value=true, err=0x7fffffffdc80) at hw/core/qdev.c:699 #4 0x0000555555849520 in property_set_bool (obj=0x5555565d3020, v=0x5555565d54d0, opaque=0x5555565ca870, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/object.c:1315 #5 0x0000555555848065 in object_property_set (obj=0x5555565d3020, v=0x5555565d54d0, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/object.c:803 #6 0x00005555558497ca in object_property_set_qobject (obj=0x5555565d3020, value=0x5555565adc20, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/qom-qobject.c:24 #7 0x0000555555848351 in object_property_set_bool (obj=0x5555565d3020, value=true, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/object.c:866 #8 0x00005555556ce60f in qdev_init (dev=0x5555565d3020) at hw/core/qdev.c:163 #9 0x00005555556ceb8e in qdev_init_nofail (dev=0x5555565d3020) at hw/core/qdev.c:277 #10 0x00005555556d11c3 in sysbus_create_varargs (name=0x555555a87f74 "pl181", addr=268455936) at hw/core/sysbus.c:157 #11 0x0000555555901572 in versatile_init (args=0x7fffffffe2d0, board_id=387) at hw/arm/versatilepb.c:284 #12 0x0000555555901835 in vpb_init (args=0x7fffffffe2d0) at hw/arm/versatilepb.c:357 #13 0x000055555589a38b in main (argc=13, argv=0x7fffffffe508, envp=0x7fffffffe578) at vl.c:4245 (gdb) c Continuing. Breakpoint 5, pl181_init (sbd=0x5555565deda0) at hw/sd/pl181.c:482 482 { (gdb) Continuing. Program received signal SIGSEGV, Segmentation fault. 0x000055555560b25f in bdrv_is_read_only (bs=0x0) at block.c:2933 2933 return bs->read_only; (gdb) i s #0 0x000055555560b25f in bdrv_is_read_only (bs=0x0) at block.c:2933 #1 0x0000555555794220 in sd_init (bs=0x0, is_spi=false) at hw/sd/sd.c:497 #2 0x000055555579316e in pl181_init (sbd=0x5555565deda0) at hw/sd/pl181.c:493 #3 0x00005555556d10e8 in sysbus_device_init (dev=0x5555565deda0) at hw/core/sysbus.c:143 #4 0x00005555556ce6d3 in device_realize (dev=0x5555565deda0, err=0x7fffffffdb08) at hw/core/qdev.c:178 #5 0x00005555556d002a in device_set_realized (obj=0x5555565deda0, value=true, err=0x7fffffffdc80) at hw/core/qdev.c:699 #6 0x0000555555849520 in property_set_bool (obj=0x5555565deda0, v=0x5555565e1250, opaque=0x5555565ca500, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/object.c:1315 #7 0x0000555555848065 in object_property_set (obj=0x5555565deda0, v=0x5555565e1250, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/object.c:803 #8 0x00005555558497ca in object_property_set_qobject (obj=0x5555565deda0, value=0x5555565ca5f0, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/qom-qobject.c:24 #9 0x0000555555848351 in object_property_set_bool (obj=0x5555565deda0, value=true, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at qom/object.c:866 #10 0x00005555556ce60f in qdev_init (dev=0x5555565deda0) at hw/core/qdev.c:163 #11 0x00005555556ceb8e in qdev_init_nofail (dev=0x5555565deda0) at hw/core/qdev.c:277 #12 0x00005555556d11c3 in sysbus_create_varargs (name=0x555555a87f74 "pl181", addr=268480512) at hw/core/sysbus.c:157 #13 0x000055555590159f in versatile_init (args=0x7fffffffe2d0, board_id=387) at hw/arm/versatilepb.c:285 #14 0x0000555555901835 in vpb_init (args=0x7fffffffe2d0) at hw/arm/versatilepb.c:357 #15 0x000055555589a38b in main (argc=13, argv=0x7fffffffe508, envp=0x7fffffffe578) at vl.c:4245
diff --git a/blockdev.c b/blockdev.c index b11155c..401ee25 100644 --- a/blockdev.c +++ b/blockdev.c @@ -528,12 +528,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, if (media == MEDIA_CDROM) { /* CDROM is fine for any interface, don't check. */ ro = 1; - } else if (ro == 1) { - if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && - type != IF_NONE && type != IF_PFLASH) { - error_report("read-only not supported by this bus type"); - goto err; - } } bdrv_flags |= ro ? 0 : BDRV_O_RDWR; diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 8c3b7f0..02a1544 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -624,6 +624,11 @@ static int m25p80_init(SSISlave *ss) if (dinfo && dinfo->bdrv) { DB_PRINT_L(0, "Binding to IF_MTD drive\n"); s->bdrv = dinfo->bdrv; + if (bdrv_is_read_only(s->bdrv)) { + fprintf(stderr, "Can't use a read-only drive"); + return 1; + } + /* FIXME: Move to late init */ if (bdrv_read(s->bdrv, 0, s->storage, DIV_ROUND_UP(s->size, BDRV_SECTOR_SIZE))) { diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 8742294..098f6c6 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -830,6 +830,11 @@ static int blk_connect(struct XenDevice *xendev) /* setup via qemu cmdline -> already setup for us */ xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n"); blkdev->bs = blkdev->dinfo->bdrv; + if (bdrv_is_read_only(blkdev->bs) && !readonly) { + xen_be_printf(&blkdev->xendev, 0, "Unexpected read-only drive"); + blkdev->bs = NULL; + return -1; + } /* blkdev->bs is not create by us, we get a reference * so we can bdrv_unref() unconditionally */ bdrv_ref(blkdev->bs); diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c index 42613b3..d1168c9 100644 --- a/hw/sd/milkymist-memcard.c +++ b/hw/sd/milkymist-memcard.c @@ -255,6 +255,10 @@ static int milkymist_memcard_init(SysBusDevice *dev) dinfo = drive_get_next(IF_SD); s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false); + if (s->card == NULL) { + return -1; + } + s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0; memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s, diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c index bf5d1fb..937a478 100644 --- a/hw/sd/omap_mmc.c +++ b/hw/sd/omap_mmc.c @@ -593,6 +593,9 @@ struct omap_mmc_s *omap_mmc_init(hwaddr base, /* Instantiate the storage */ s->card = sd_init(bd, false); + if (s->card == NULL) { + exit(1); + } return s; } @@ -618,6 +621,9 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta, /* Instantiate the storage */ s->card = sd_init(bd, false); + if (s->card == NULL) { + exit(1); + } s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0]; sd_set_cb(s->card, NULL, s->cdet); diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c index 03875bf..c35896d 100644 --- a/hw/sd/pl181.c +++ b/hw/sd/pl181.c @@ -491,6 +491,10 @@ static int pl181_init(SysBusDevice *sbd) qdev_init_gpio_out(dev, s->cardstatus, 2); dinfo = drive_get_next(IF_SD); s->card = sd_init(dinfo ? dinfo->bdrv : NULL, false); + if (s->card == NULL) { + return -1; + } + return 0; } diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c index 90c955f..b9d8b1a 100644 --- a/hw/sd/pxa2xx_mmci.c +++ b/hw/sd/pxa2xx_mmci.c @@ -539,6 +539,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem, /* Instantiate the actual storage */ s->card = sd_init(bd, false); + if (s->card == NULL) { + exit(1); + } register_savevm(NULL, "pxa2xx_mmci", 0, 0, pxa2xx_mmci_save, pxa2xx_mmci_load, s); diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 346d86f..7380f06 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -494,6 +494,11 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi) { SDState *sd; + if (bdrv_is_read_only(bs)) { + fprintf(stderr, "sd_init: Cannot use read-only drive\n"); + return NULL; + } + sd = (SDState *) g_malloc0(sizeof(SDState)); sd->buf = qemu_blockalign(bs, 512); sd->spi = is_spi; diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 1483e19..0906a1d 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -1166,6 +1166,9 @@ static void sdhci_initfn(Object *obj) di = drive_get_next(IF_SD); s->card = sd_init(di ? di->bdrv : NULL, false); + if (s->card == NULL) { + exit(1); + } s->eject_cb = qemu_allocate_irqs(sdhci_insert_eject_cb, s, 1)[0]; s->ro_cb = qemu_allocate_irqs(sdhci_card_readonly_cb, s, 1)[0]; sd_set_cb(s->card, s->ro_cb, s->eject_cb); diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index d47e237..1bb56c4 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -246,6 +246,9 @@ static int ssi_sd_init(SSISlave *dev) s->mode = SSI_SD_CMD; dinfo = drive_get_next(IF_SD); s->sd = sd_init(dinfo ? dinfo->bdrv : NULL, true); + if (s->sd == NULL) { + return -1; + } register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s); return 0; } diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index e58776a..2839e32 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -139,7 +139,10 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) q[K[Dqu[K[D[Dqui[K[D[D[Dquit[K Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on: read-only not supported by this bus type +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) QEMU_PROG: Can't use a read-only drive +QEMU_PROG: Device initialization failed. +QEMU_PROG: Initialization of device ide-hd failed Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on QEMU X.Y.Z monitor - type 'help' for more information