* [PATCH v3 1/3] hw/arm/boot: Abort if set_kernel_args() fails
2021-05-20 5:15 [PATCH v3 0/3] exec/memory: Enforce checking MemTxResult values Philippe Mathieu-Daudé
@ 2021-05-20 5:15 ` Philippe Mathieu-Daudé
2021-05-27 9:04 ` Peter Maydell
2021-05-20 5:15 ` [PATCH v3 2/3] hw/core/loader: Move 'write_rom' trace event earlier Philippe Mathieu-Daudé
2021-05-20 5:15 ` [PATCH v3 3/3] hw/core/loader: Warn if we fail to load ROM regions at reset Philippe Mathieu-Daudé
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-20 5:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
If a address_space_write() call fails while calling
set_kernel_args(), the guest kernel will boot using
crap data. Avoid that by aborting if this ever occurs.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/arm/boot.c | 53 ++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index d7b059225e6..0c1346d5842 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -291,15 +291,20 @@ static inline bool have_dtb(const struct arm_boot_info *info)
#define WRITE_WORD(p, value) do { \
address_space_stl_notdirty(as, p, value, \
- MEMTXATTRS_UNSPECIFIED, NULL); \
+ MEMTXATTRS_UNSPECIFIED, &result); \
+ if (result != MEMTX_OK) { \
+ goto fail; \
+ } \
p += 4; \
} while (0)
-static void set_kernel_args(const struct arm_boot_info *info, AddressSpace *as)
+/* Returns: 0 on success, -1 on error */
+static int set_kernel_args(const struct arm_boot_info *info, AddressSpace *as)
{
int initrd_size = info->initrd_size;
hwaddr base = info->loader_start;
hwaddr p;
+ MemTxResult result;
p = base + KERNEL_ARGS_ADDR;
/* ATAG_CORE */
@@ -326,8 +331,11 @@ static void set_kernel_args(const struct arm_boot_info *info, AddressSpace *as)
int cmdline_size;
cmdline_size = strlen(info->kernel_cmdline);
- address_space_write(as, p + 8, MEMTXATTRS_UNSPECIFIED,
- info->kernel_cmdline, cmdline_size + 1);
+ result = address_space_write(as, p + 8, MEMTXATTRS_UNSPECIFIED,
+ info->kernel_cmdline, cmdline_size + 1);
+ if (result != MEMTX_OK) {
+ goto fail;
+ }
cmdline_size = (cmdline_size >> 2) + 1;
WRITE_WORD(p, cmdline_size + 2);
WRITE_WORD(p, 0x54410009);
@@ -341,22 +349,31 @@ static void set_kernel_args(const struct arm_boot_info *info, AddressSpace *as)
atag_board_len = (info->atag_board(info, atag_board_buf) + 3) & ~3;
WRITE_WORD(p, (atag_board_len + 8) >> 2);
WRITE_WORD(p, 0x414f4d50);
- address_space_write(as, p, MEMTXATTRS_UNSPECIFIED,
- atag_board_buf, atag_board_len);
+ result = address_space_write(as, p, MEMTXATTRS_UNSPECIFIED,
+ atag_board_buf, atag_board_len);
+ if (result != MEMTX_OK) {
+ goto fail;
+ }
p += atag_board_len;
}
/* ATAG_END */
WRITE_WORD(p, 0);
WRITE_WORD(p, 0);
+
+ return 0;
+fail:
+ return -1;
}
-static void set_kernel_args_old(const struct arm_boot_info *info,
- AddressSpace *as)
+/* Returns: 0 on success, -1 on error */
+static int set_kernel_args_old(const struct arm_boot_info *info,
+ AddressSpace *as)
{
hwaddr p;
const char *s;
int initrd_size = info->initrd_size;
hwaddr base = info->loader_start;
+ MemTxResult result;
/* see linux/include/asm-arm/setup.h */
p = base + KERNEL_ARGS_ADDR;
@@ -419,10 +436,18 @@ static void set_kernel_args_old(const struct arm_boot_info *info,
}
s = info->kernel_cmdline;
if (s) {
- address_space_write(as, p, MEMTXATTRS_UNSPECIFIED, s, strlen(s) + 1);
+ result = address_space_write(as, p, MEMTXATTRS_UNSPECIFIED,
+ s, strlen(s) + 1);
+ if (result != MEMTX_OK) {
+ goto fail;
+ }
} else {
WRITE_WORD(p, 0);
}
+
+ return 0;
+fail:
+ return -1;
}
static int fdt_add_memory_node(void *fdt, uint32_t acells, hwaddr mem_base,
@@ -786,10 +811,16 @@ static void do_cpu_reset(void *opaque)
cpu_set_pc(cs, info->loader_start);
if (!have_dtb(info)) {
+ int err;
+
if (old_param) {
- set_kernel_args_old(info, as);
+ err = set_kernel_args_old(info, as);
} else {
- set_kernel_args(info, as);
+ err = set_kernel_args(info, as);
+ }
+ if (err) {
+ error_report("could not set kernel arguments");
+ exit(1);
}
}
} else {
--
2.26.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] hw/core/loader: Move 'write_rom' trace event earlier
2021-05-20 5:15 [PATCH v3 0/3] exec/memory: Enforce checking MemTxResult values Philippe Mathieu-Daudé
2021-05-20 5:15 ` [PATCH v3 1/3] hw/arm/boot: Abort if set_kernel_args() fails Philippe Mathieu-Daudé
@ 2021-05-20 5:15 ` Philippe Mathieu-Daudé
2021-05-20 5:15 ` [PATCH v3 3/3] hw/core/loader: Warn if we fail to load ROM regions at reset Philippe Mathieu-Daudé
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-20 5:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
It is more useful to trace the event which will happen,
rather than missing an event that failed. So move the
'write_rom' trace event earlier.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/core/loader.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 5b34869a541..b3c4a654b45 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1142,6 +1142,7 @@ static void rom_reset(void *unused)
if (rom->data == NULL) {
continue;
}
+ trace_loader_write_rom(rom->name, rom->addr, rom->datasize, rom->isrom);
if (rom->mr) {
void *host = memory_region_get_ram_ptr(rom->mr);
memcpy(host, rom->data, rom->datasize);
@@ -1160,8 +1161,6 @@ static void rom_reset(void *unused)
* CPU definitely fetches its instructions from the just written data.
*/
cpu_flush_icache_range(rom->addr, rom->datasize);
-
- trace_loader_write_rom(rom->name, rom->addr, rom->datasize, rom->isrom);
}
}
--
2.26.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] hw/core/loader: Warn if we fail to load ROM regions at reset
2021-05-20 5:15 [PATCH v3 0/3] exec/memory: Enforce checking MemTxResult values Philippe Mathieu-Daudé
2021-05-20 5:15 ` [PATCH v3 1/3] hw/arm/boot: Abort if set_kernel_args() fails Philippe Mathieu-Daudé
2021-05-20 5:15 ` [PATCH v3 2/3] hw/core/loader: Move 'write_rom' trace event earlier Philippe Mathieu-Daudé
@ 2021-05-20 5:15 ` Philippe Mathieu-Daudé
2021-05-27 9:07 ` Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-20 5:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
If the user provides an ELF file that's been linked to a wrong
address, we try to load it, fails, and keep going silently.
Instead,
Display a warning instead, but keep going to not disrupt users
accidentally relying on this 'continues-anyway' behaviour.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/core/loader.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index b3c4a654b45..37a2f2c4959 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1147,8 +1147,16 @@ static void rom_reset(void *unused)
void *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,
- rom->data, rom->datasize);
+ MemTxResult res;
+
+ res = address_space_write_rom(rom->as, rom->addr,
+ MEMTXATTRS_UNSPECIFIED,
+ rom->data, rom->datasize);
+ if (res != MEMTX_OK) {
+ warn_report("rom: unable to write data (file '%s', "
+ "addr=0x" TARGET_FMT_plx ", size=0x%zu)",
+ rom->name, rom->addr, rom->datasize);
+ }
}
if (rom->isrom) {
/* rom needs to be written only once */
--
2.26.3
^ permalink raw reply related [flat|nested] 6+ messages in thread