[Qemu-devel] [PATCH 0/2] pci: handle zero size ROM files gracefully

[Qemu-devel] [PATCH 0/2] pci: handle zero size ROM files gracefully

[Stefan Hajnoczi]

This is a fix for https://bugs.launchpad.net/qemu/+bug/1127053.

If pxe-e1000.rom has 0 size we allocate the same RAMBlock offset twice and fail
with an assertion in qemu_ram_set_idstr() later on.

Two fixes:

1. Print an error when the ROM file has zero size.  Use -device ...,romfile=
   instead to disable the ROM.

2. Add an assertion to find_ram_offset() so we watch this inconsistency if it
   ever happens again.

Stefan Hajnoczi (2):
  pci: refuse empty ROM files
  exec: assert that RAMBlock size is non-zero

 exec.c       | 2 ++
 hw/pci/pci.c | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/2] pci: refuse empty ROM files

[Stefan Hajnoczi]

A zero size ROM file is invalid and should produce a warning.
Attempting to use a zero size file ends up hitting an assertion
qemu_ram_set_idstr() because RAMBlocks with duplicate addresses are
allocated - due to zero size the allocator doesn't increment the next
available RAMBlock offset.

Also convert __FUNCTION__ to __func__ while we're touching this code.
There are no other __FUNCTION__ instances in pci.c anymore.

Reported-by: Milos Ivanovic <milosivanovic@orcon.net.nz>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/pci/pci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2f45c8f..9d5907c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1852,7 +1852,12 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
     size = get_image_size(path);
     if (size < 0) {
         error_report("%s: failed to find romfile \"%s\"",
-                     __FUNCTION__, pdev->romfile);
+                     __func__, pdev->romfile);
+        g_free(path);
+        return -1;
+    } else if (size == 0) {
+        error_report("%s: ignoring empty romfile \"%s\"",
+                     __func__, pdev->romfile);
         g_free(path);
         return -1;
     }
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/2] exec: assert that RAMBlock size is non-zero

[Stefan Hajnoczi]

find_ram_offset() does not handle size=0 gracefully.  It hands out the
same RAMBlock offset multiple times, leading to obscure failures later
on.

Add an assert to warn early if something is incorrectly allocating a
zero size RAMBlock.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 exec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/exec.c b/exec.c
index 46a2830..a9aa703 100644
--- a/exec.c
+++ b/exec.c
@@ -912,6 +912,8 @@ static ram_addr_t find_ram_offset(ram_addr_t size)
     RAMBlock *block, *next_block;
     ram_addr_t offset = RAM_ADDR_MAX, mingap = RAM_ADDR_MAX;
 
+    assert(size != 0); /* it would hand out same offset multiple times */
+
     if (QTAILQ_EMPTY(&ram_list.blocks))
         return 0;
 
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 0/2] pci: handle zero size ROM files gracefully

[Michael S. Tsirkin]

On Mon, Mar 11, 2013 at 10:20:19AM +0100, Stefan Hajnoczi wrote:
> This is a fix for https://bugs.launchpad.net/qemu/+bug/1127053.
> 
> If pxe-e1000.rom has 0 size we allocate the same RAMBlock offset twice and fail
> with an assertion in qemu_ram_set_idstr() later on.
> 
> Two fixes:
> 
> 1. Print an error when the ROM file has zero size.  Use -device ...,romfile=
>    instead to disable the ROM.
> 
> 2. Add an assertion to find_ram_offset() so we watch this inconsistency if it
>    ever happens again.
> 
> Stefan Hajnoczi (2):
>   pci: refuse empty ROM files
>   exec: assert that RAMBlock size is non-zero
> 
>  exec.c       | 2 ++
>  hw/pci/pci.c | 7 ++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)

Applied, thanks.

> -- 
> 1.8.1.4