* [PATCH 1/5] hw/sparc64/niagara: Pass available memory region size to add_rom_or_fail
2020-03-09 14:43 [PATCH 0/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
@ 2020-03-09 14:43 ` Philippe Mathieu-Daudé
2020-03-09 14:43 ` [PATCH 2/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 14:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, qemu-ppc, Gerd Hoffmann, Paolo Bonzini,
David Gibson, Philippe Mathieu-Daudé, Artyom Tarasenko,
Richard Henderson
In few commits we'll let rom_add_file_fixed() take a 'size'
argument holding the maximum file length that can be loaded
as ROM. In preparation, modify the Niagara machine, so the
generic change will be easier to review.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/sparc64/niagara.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
index ab5ef8c5b3..f58d008d3d 100644
--- a/hw/sparc64/niagara.c
+++ b/hw/sparc64/niagara.c
@@ -85,7 +85,8 @@ typedef struct NiagaraBoardState {
#define NIAGARA_OBP_OFFSET 0x80000ULL
#define PROM_SIZE_MAX (4 * MiB)
-static void add_rom_or_fail(const char *file, const hwaddr addr)
+static void add_rom_or_fail(const char *file, const hwaddr addr,
+ uint64_t region_size)
{
/* XXX remove qtest_enabled() check once firmware files are
* in the qemu tree
@@ -126,13 +127,15 @@ static void niagara_init(MachineState *machine)
&error_fatal);
memory_region_add_subregion(sysmem, NIAGARA_PROM_BASE, &s->prom);
- add_rom_or_fail("nvram1", NIAGARA_NVRAM_BASE);
- add_rom_or_fail("1up-md.bin", NIAGARA_MD_ROM_BASE);
- add_rom_or_fail("1up-hv.bin", NIAGARA_HV_ROM_BASE);
+ add_rom_or_fail("nvram1", NIAGARA_NVRAM_BASE, NIAGARA_NVRAM_SIZE);
+ add_rom_or_fail("1up-md.bin", NIAGARA_MD_ROM_BASE, NIAGARA_MD_ROM_SIZE);
+ add_rom_or_fail("1up-hv.bin", NIAGARA_HV_ROM_BASE, NIAGARA_HV_ROM_SIZE);
- add_rom_or_fail("reset.bin", NIAGARA_PROM_BASE);
- add_rom_or_fail("q.bin", NIAGARA_PROM_BASE + NIAGARA_Q_OFFSET);
- add_rom_or_fail("openboot.bin", NIAGARA_PROM_BASE + NIAGARA_OBP_OFFSET);
+ add_rom_or_fail("reset.bin", NIAGARA_PROM_BASE, NIAGARA_Q_OFFSET);
+ add_rom_or_fail("q.bin", NIAGARA_PROM_BASE + NIAGARA_Q_OFFSET,
+ NIAGARA_OBP_OFFSET);
+ add_rom_or_fail("openboot.bin", NIAGARA_PROM_BASE + NIAGARA_OBP_OFFSET,
+ PROM_SIZE_MAX - NIAGARA_OBP_OFFSET);
/* the virtual ramdisk is kind of initrd, but it resides
outside of the partition RAM */
--
2.21.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] hw/core/loader: Prohibit loading ROMs bigger than memory region
2020-03-09 14:43 [PATCH 0/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
2020-03-09 14:43 ` [PATCH 1/5] hw/sparc64/niagara: Pass available memory region size to add_rom_or_fail Philippe Mathieu-Daudé
@ 2020-03-09 14:43 ` Philippe Mathieu-Daudé
2020-03-09 14:48 ` Peter Maydell
2020-03-09 14:43 ` [PATCH 3/5] hw/core/loader: Provide rom_add_file() a 'max_size' argument Philippe Mathieu-Daudé
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 14:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, qemu-ppc, Gerd Hoffmann, Paolo Bonzini,
David Gibson, Philippe Mathieu-Daudé, Artyom Tarasenko,
Richard Henderson
We must not write more data than the memory region size.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/core/loader.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index d1b78f60cd..c67c483936 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1136,7 +1136,10 @@ static void rom_reset(void *unused)
continue;
}
if (rom->mr) {
- void *host = memory_region_get_ram_ptr(rom->mr);
+ void *host;
+
+ assert(memory_region_size(rom->mr) >= rom->datasize);
+ host = memory_region_get_ram_ptr(rom->mr);
memcpy(host, rom->data, rom->datasize);
} else {
address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
--
2.21.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/5] hw/core/loader: Prohibit loading ROMs bigger than memory region
2020-03-09 14:43 ` [PATCH 2/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
@ 2020-03-09 14:48 ` Peter Maydell
2020-03-09 15:41 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2020-03-09 14:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, QEMU Developers, qemu-ppc, Gerd Hoffmann,
Paolo Bonzini, Richard Henderson, Artyom Tarasenko, David Gibson
On Mon, 9 Mar 2020 at 14:45, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> We must not write more data than the memory region size.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/core/loader.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index d1b78f60cd..c67c483936 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1136,7 +1136,10 @@ static void rom_reset(void *unused)
> continue;
> }
> if (rom->mr) {
> - void *host = memory_region_get_ram_ptr(rom->mr);
> + void *host;
> +
> + assert(memory_region_size(rom->mr) >= rom->datasize);
> + host = memory_region_get_ram_ptr(rom->mr);
> memcpy(host, rom->data, rom->datasize);
Does this really only happen if there's a QEMU bug,
or could a user trigger this assert by accidentally
passing an oversize file on the command line?
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/5] hw/core/loader: Prohibit loading ROMs bigger than memory region
2020-03-09 14:48 ` Peter Maydell
@ 2020-03-09 15:41 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 15:41 UTC (permalink / raw)
To: Peter Maydell
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, QEMU Developers, qemu-ppc, Gerd Hoffmann,
Paolo Bonzini, Richard Henderson, Artyom Tarasenko, David Gibson
On 3/9/20 3:48 PM, Peter Maydell wrote:
> On Mon, 9 Mar 2020 at 14:45, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> We must not write more data than the memory region size.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> hw/core/loader.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/loader.c b/hw/core/loader.c
>> index d1b78f60cd..c67c483936 100644
>> --- a/hw/core/loader.c
>> +++ b/hw/core/loader.c
>> @@ -1136,7 +1136,10 @@ static void rom_reset(void *unused)
>> continue;
>> }
>> if (rom->mr) {
>> - void *host = memory_region_get_ram_ptr(rom->mr);
>> + void *host;
>> +
>> + assert(memory_region_size(rom->mr) >= rom->datasize);
>> + host = memory_region_get_ram_ptr(rom->mr);
>> memcpy(host, rom->data, rom->datasize);
>
> Does this really only happen if there's a QEMU bug,
> or could a user trigger this assert by accidentally
> passing an oversize file on the command line?
This is definitively command-line triggered, but it occurred when I was
trying the RX port (not yet merged) so the bug might be an incorrect API
use there. I'll check tonight. Meanwhile I went consolidating the rest
of the code to feel safer.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/5] hw/core/loader: Provide rom_add_file() a 'max_size' argument
2020-03-09 14:43 [PATCH 0/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
2020-03-09 14:43 ` [PATCH 1/5] hw/sparc64/niagara: Pass available memory region size to add_rom_or_fail Philippe Mathieu-Daudé
2020-03-09 14:43 ` [PATCH 2/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
@ 2020-03-09 14:43 ` Philippe Mathieu-Daudé
2020-03-09 17:44 ` Taylor Simpson
2020-03-09 14:43 ` [PATCH 4/5] hw/core/loader: Restrict rom_add_file_mr() to available region size Philippe Mathieu-Daudé
2020-03-09 14:43 ` [PATCH 5/5] hw/core/loader: Provide rom_add_file_fixed() a 'max_size' argument Philippe Mathieu-Daudé
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 14:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, qemu-ppc, Gerd Hoffmann, Paolo Bonzini,
David Gibson, Philippe Mathieu-Daudé, Artyom Tarasenko,
Richard Henderson
In commit d60fa42e8b we cared about loading ROMs smaller than
the underlying memory region (by zeroing the missing area) but
we forgot the case where a ROM file is bigger than the underlying
region.
Provide rom_add_file() a 'max_size' argument, and refuse loading
ROM files bigger than it.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/loader.h | 12 ++++++------
hw/core/loader.c | 19 +++++++++++++------
2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index a9eeea3952..99d690a628 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -254,7 +254,7 @@ extern bool option_rom_has_mr;
extern bool rom_file_has_mr;
int rom_add_file(const char *file, const char *fw_dir,
- hwaddr addr, int32_t bootindex,
+ hwaddr addr, uint64_t max_len, int32_t bootindex,
bool option_rom, MemoryRegion *mr, AddressSpace *as);
MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
size_t max_len, hwaddr addr,
@@ -292,16 +292,16 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size);
void *rom_ptr(hwaddr addr, size_t size);
void hmp_info_roms(Monitor *mon, const QDict *qdict);
-#define rom_add_file_fixed(_f, _a, _i) \
- rom_add_file(_f, NULL, _a, _i, false, NULL, NULL)
+#define rom_add_file_fixed(_f, _a, _i) \
+ rom_add_file(_f, NULL, _a, -1, _i, false, NULL, NULL)
#define rom_add_blob_fixed(_f, _b, _l, _a) \
rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL, true)
#define rom_add_file_mr(_f, _mr, _i) \
- rom_add_file(_f, NULL, 0, _i, false, _mr, NULL)
+ rom_add_file(_f, NULL, 0, -1, _i, false, _mr, NULL)
#define rom_add_file_as(_f, _as, _i) \
- rom_add_file(_f, NULL, 0, _i, false, NULL, _as)
+ rom_add_file(_f, NULL, 0, -1, _i, false, NULL, _as)
#define rom_add_file_fixed_as(_f, _a, _i, _as) \
- rom_add_file(_f, NULL, _a, _i, false, NULL, _as)
+ rom_add_file(_f, NULL, _a, -1, _i, false, NULL, _as)
#define rom_add_blob_fixed_as(_f, _b, _l, _a, _as) \
rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as, true)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index c67c483936..00436f1524 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -938,7 +938,7 @@ static void *rom_set_mr(Rom *rom, Object *owner, const char *name, bool ro)
}
int rom_add_file(const char *file, const char *fw_dir,
- hwaddr addr, int32_t bootindex,
+ hwaddr addr, uint64_t max_len, int32_t bootindex,
bool option_rom, MemoryRegion *mr,
AddressSpace *as)
{
@@ -974,14 +974,21 @@ int rom_add_file(const char *file, const char *fw_dir,
rom->fw_file = g_strdup(file);
}
rom->addr = addr;
- rom->romsize = lseek(fd, 0, SEEK_END);
- if (rom->romsize == -1) {
+ rom->datasize = lseek(fd, 0, SEEK_END);
+ if (rom->datasize == -1) {
fprintf(stderr, "rom: file %-20s: get size error: %s\n",
rom->name, strerror(errno));
goto err;
}
- rom->datasize = rom->romsize;
+ rom->romsize = max_len ? max_len : rom->datasize;
+ if (rom->romsize < rom->datasize) {
+ fprintf(stderr,
+ "rom: file '%-20s' size too big: %zu (available: %zu)\n",
+ rom->name, rom->datasize, rom->romsize);
+ goto err;
+ }
+
rom->data = g_malloc0(rom->datasize);
lseek(fd, 0, SEEK_SET);
rc = read(fd, rom->data, rom->datasize);
@@ -1107,12 +1114,12 @@ int rom_add_elf_program(const char *name, GMappedFile *mapped_file, void *data,
int rom_add_vga(const char *file)
{
- return rom_add_file(file, "vgaroms", 0, -1, true, NULL, NULL);
+ return rom_add_file(file, "vgaroms", 0, -1, -1, true, NULL, NULL);
}
int rom_add_option(const char *file, int32_t bootindex)
{
- return rom_add_file(file, "genroms", 0, bootindex, true, NULL, NULL);
+ return rom_add_file(file, "genroms", 0, -1, bootindex, true, NULL, NULL);
}
static void rom_reset(void *unused)
--
2.21.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* RE: [PATCH 3/5] hw/core/loader: Provide rom_add_file() a 'max_size' argument
2020-03-09 14:43 ` [PATCH 3/5] hw/core/loader: Provide rom_add_file() a 'max_size' argument Philippe Mathieu-Daudé
@ 2020-03-09 17:44 ` Taylor Simpson
0 siblings, 0 replies; 10+ messages in thread
From: Taylor Simpson @ 2020-03-09 17:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel@nongnu.org
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, qemu-ppc@nongnu.org, Gerd Hoffmann,
Paolo Bonzini, Richard Henderson, Artyom Tarasenko, David Gibson
> -----Original Message-----
> From: Qemu-devel <qemu-devel-
> bounces+tsimpson=quicinc.com@nongnu.org> On Behalf Of Philippe
> Mathieu-Daudé
> Sent: Monday, March 9, 2020 9:44 AM
> To: qemu-devel@nongnu.org
> Cc: Eduardo Habkost <ehabkost@redhat.com>; Michael S. Tsirkin
> <mst@redhat.com>; Alistair Francis <alistair@alistair23.me>; Mark Cave-
> Ayland <mark.cave-ayland@ilande.co.uk>; qemu-ppc@nongnu.org; Gerd
> Hoffmann <kraxel@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>;
> David Gibson <david@gibson.dropbear.id.au>; Philippe Mathieu-Daudé
> <philmd@redhat.com>; Artyom Tarasenko <atar4qemu@gmail.com>;
> Richard Henderson <rth@twiddle.net>
> Subject: [PATCH 3/5] hw/core/loader: Provide rom_add_file() a 'max_size'
> argument
>
>
> -#define rom_add_file_fixed(_f, _a, _i) \
> - rom_add_file(_f, NULL, _a, _i, false, NULL, NULL)
> +#define rom_add_file_fixed(_f, _a, _i) \
> + rom_add_file(_f, NULL, _a, -1, _i, false, NULL, NULL)
> #define rom_add_blob_fixed(_f, _b, _l, _a) \
> rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL, true)
> #define rom_add_file_mr(_f, _mr, _i) \
> - rom_add_file(_f, NULL, 0, _i, false, _mr, NULL)
> + rom_add_file(_f, NULL, 0, -1, _i, false, _mr, NULL)
> #define rom_add_file_as(_f, _as, _i) \
> - rom_add_file(_f, NULL, 0, _i, false, NULL, _as)
> + rom_add_file(_f, NULL, 0, -1, _i, false, NULL, _as)
> #define rom_add_file_fixed_as(_f, _a, _i, _as) \
> - rom_add_file(_f, NULL, _a, _i, false, NULL, _as)
> + rom_add_file(_f, NULL, _a, -1, _i, false, NULL, _as)
> #define rom_add_blob_fixed_as(_f, _b, _l, _a, _as) \
> rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, _as, true)
The above looks like -1 represents an unknown max_len.
> + rom->romsize = max_len ? max_len : rom->datasize;
So, shouldn't this check max_len == -1? In general, wouldn't it be better to #define UNKNOWN_MAX_LEN to make it more clear what the value is used for?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] hw/core/loader: Restrict rom_add_file_mr() to available region size
2020-03-09 14:43 [PATCH 0/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-03-09 14:43 ` [PATCH 3/5] hw/core/loader: Provide rom_add_file() a 'max_size' argument Philippe Mathieu-Daudé
@ 2020-03-09 14:43 ` Philippe Mathieu-Daudé
2020-03-09 14:43 ` [PATCH 5/5] hw/core/loader: Provide rom_add_file_fixed() a 'max_size' argument Philippe Mathieu-Daudé
4 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 14:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, qemu-ppc, Gerd Hoffmann, Paolo Bonzini,
David Gibson, Philippe Mathieu-Daudé, Artyom Tarasenko,
Richard Henderson
Use the ''max_size' argument recently added to rom_add_file() to
not load ROMs bigger than the underlying memory region.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/loader.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 99d690a628..34ac630eb1 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -297,7 +297,7 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict);
#define rom_add_blob_fixed(_f, _b, _l, _a) \
rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL, true)
#define rom_add_file_mr(_f, _mr, _i) \
- rom_add_file(_f, NULL, 0, -1, _i, false, _mr, NULL)
+ rom_add_file(_f, NULL, 0, memory_region_size(_mr), _i, false, _mr, NULL)
#define rom_add_file_as(_f, _as, _i) \
rom_add_file(_f, NULL, 0, -1, _i, false, NULL, _as)
#define rom_add_file_fixed_as(_f, _a, _i, _as) \
--
2.21.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] hw/core/loader: Provide rom_add_file_fixed() a 'max_size' argument
2020-03-09 14:43 [PATCH 0/5] hw/core/loader: Prohibit loading ROMs bigger than memory region Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-03-09 14:43 ` [PATCH 4/5] hw/core/loader: Restrict rom_add_file_mr() to available region size Philippe Mathieu-Daudé
@ 2020-03-09 14:43 ` Philippe Mathieu-Daudé
2020-03-10 0:43 ` David Gibson
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-09 14:43 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, qemu-ppc, Gerd Hoffmann, Paolo Bonzini,
David Gibson, Philippe Mathieu-Daudé, Artyom Tarasenko,
Richard Henderson
Let rom_add_file_fixed() call rom_add_file() with a 'max_size'
argument, to avoid writing more than the available space for
the ROMs.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/loader.h | 4 ++--
hw/i386/x86.c | 2 +-
hw/ppc/sam460ex.c | 2 +-
hw/sparc64/niagara.c | 5 +++--
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 34ac630eb1..30ed80128e 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -292,8 +292,8 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size);
void *rom_ptr(hwaddr addr, size_t size);
void hmp_info_roms(Monitor *mon, const QDict *qdict);
-#define rom_add_file_fixed(_f, _a, _i) \
- rom_add_file(_f, NULL, _a, -1, _i, false, NULL, NULL)
+#define rom_add_file_fixed(_f, _a, _ms, _i) \
+ rom_add_file(_f, NULL, _a, _ms, _i, false, NULL, NULL)
#define rom_add_blob_fixed(_f, _b, _l, _a) \
rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL, true)
#define rom_add_file_mr(_f, _mr, _i) \
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 7f38e6ba8b..bdac66206a 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -783,7 +783,7 @@ void x86_bios_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
if (!isapc_ram_fw) {
memory_region_set_readonly(bios, true);
}
- ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
+ ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), bios_size, -1);
if (ret != 0) {
bios_error:
fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 898453cf30..5eab479ae5 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -127,7 +127,7 @@ static int sam460ex_load_uboot(void)
" using default u-boot image");*/
rom_add_file_fixed(UBOOT_FILENAME,
UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32),
- -1);
+ UBOOT_SIZE, -1);
}
return 0;
diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
index f58d008d3d..4dd9a77dcb 100644
--- a/hw/sparc64/niagara.c
+++ b/hw/sparc64/niagara.c
@@ -91,7 +91,7 @@ static void add_rom_or_fail(const char *file, const hwaddr addr,
/* XXX remove qtest_enabled() check once firmware files are
* in the qemu tree
*/
- if (!qtest_enabled() && rom_add_file_fixed(file, addr, -1)) {
+ if (!qtest_enabled() && rom_add_file_fixed(file, addr, region_size, -1)) {
error_report("Unable to load a firmware for -M niagara");
exit(1);
}
@@ -148,7 +148,8 @@ static void niagara_init(MachineState *machine)
memory_region_add_subregion(get_system_memory(),
NIAGARA_VDISK_BASE, &s->vdisk_ram);
dinfo->is_default = 1;
- rom_add_file_fixed(blk_bs(blk)->filename, NIAGARA_VDISK_BASE, -1);
+ rom_add_file_fixed(blk_bs(blk)->filename, NIAGARA_VDISK_BASE,
+ size, -1);
} else {
error_report("could not load ram disk '%s'",
blk_bs(blk)->filename);
--
2.21.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 5/5] hw/core/loader: Provide rom_add_file_fixed() a 'max_size' argument
2020-03-09 14:43 ` [PATCH 5/5] hw/core/loader: Provide rom_add_file_fixed() a 'max_size' argument Philippe Mathieu-Daudé
@ 2020-03-10 0:43 ` David Gibson
0 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2020-03-10 0:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, Michael S. Tsirkin, Alistair Francis,
Mark Cave-Ayland, qemu-devel, qemu-ppc, Gerd Hoffmann,
Paolo Bonzini, Artyom Tarasenko, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 3786 bytes --]
On Mon, Mar 09, 2020 at 03:43:53PM +0100, Philippe Mathieu-Daudé wrote:
> Let rom_add_file_fixed() call rom_add_file() with a 'max_size'
> argument, to avoid writing more than the available space for
> the ROMs.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
ppc parts
Acked-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> include/hw/loader.h | 4 ++--
> hw/i386/x86.c | 2 +-
> hw/ppc/sam460ex.c | 2 +-
> hw/sparc64/niagara.c | 5 +++--
> 4 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 34ac630eb1..30ed80128e 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -292,8 +292,8 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size);
> void *rom_ptr(hwaddr addr, size_t size);
> void hmp_info_roms(Monitor *mon, const QDict *qdict);
>
> -#define rom_add_file_fixed(_f, _a, _i) \
> - rom_add_file(_f, NULL, _a, -1, _i, false, NULL, NULL)
> +#define rom_add_file_fixed(_f, _a, _ms, _i) \
> + rom_add_file(_f, NULL, _a, _ms, _i, false, NULL, NULL)
> #define rom_add_blob_fixed(_f, _b, _l, _a) \
> rom_add_blob(_f, _b, _l, _l, _a, NULL, NULL, NULL, NULL, true)
> #define rom_add_file_mr(_f, _mr, _i) \
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 7f38e6ba8b..bdac66206a 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -783,7 +783,7 @@ void x86_bios_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
> if (!isapc_ram_fw) {
> memory_region_set_readonly(bios, true);
> }
> - ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
> + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), bios_size, -1);
> if (ret != 0) {
> bios_error:
> fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name);
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 898453cf30..5eab479ae5 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -127,7 +127,7 @@ static int sam460ex_load_uboot(void)
> " using default u-boot image");*/
> rom_add_file_fixed(UBOOT_FILENAME,
> UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32),
> - -1);
> + UBOOT_SIZE, -1);
> }
>
> return 0;
> diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
> index f58d008d3d..4dd9a77dcb 100644
> --- a/hw/sparc64/niagara.c
> +++ b/hw/sparc64/niagara.c
> @@ -91,7 +91,7 @@ static void add_rom_or_fail(const char *file, const hwaddr addr,
> /* XXX remove qtest_enabled() check once firmware files are
> * in the qemu tree
> */
> - if (!qtest_enabled() && rom_add_file_fixed(file, addr, -1)) {
> + if (!qtest_enabled() && rom_add_file_fixed(file, addr, region_size, -1)) {
> error_report("Unable to load a firmware for -M niagara");
> exit(1);
> }
> @@ -148,7 +148,8 @@ static void niagara_init(MachineState *machine)
> memory_region_add_subregion(get_system_memory(),
> NIAGARA_VDISK_BASE, &s->vdisk_ram);
> dinfo->is_default = 1;
> - rom_add_file_fixed(blk_bs(blk)->filename, NIAGARA_VDISK_BASE, -1);
> + rom_add_file_fixed(blk_bs(blk)->filename, NIAGARA_VDISK_BASE,
> + size, -1);
> } else {
> error_report("could not load ram disk '%s'",
> blk_bs(blk)->filename);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread