Hi,
Le 26/07/2015 22:11, Aurelien Jarno a écrit :
On 2015-07-24 20:42, Hervé Poussineau wrote:
This fixes a guest-triggerable QEMU crash when guest tries to write to PROM.
Signed-off-by: Hervé Poussineau <hpous...@reactos.org>
---
hw/net/dp8393x.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 8fafdb0..55168b5 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -601,6 +601,16 @@ static const MemoryRegionOps dp8393x_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+static bool dp8393x_rom_accepts(void *opaque, hwaddr addr, unsigned int size,
+ bool is_write)
+{
+ return !is_write;
+}
+
+static const MemoryRegionOps dp8393x_rom_ops = {
+ .valid.accepts = dp8393x_rom_accepts,
+};
+
static void dp8393x_watchdog(void *opaque)
{
dp8393xState *s = opaque;
@@ -840,7 +850,7 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
- memory_region_init_rom_device(&s->prom, OBJECT(dev), NULL, NULL,
+ memory_region_init_rom_device(&s->prom, OBJECT(dev), &dp8393x_rom_ops,
NULL,
"dp8393x-prom", SONIC_PROM_SIZE, NULL);
prom = memory_region_get_ram_ptr(&s->prom);
checksum = 0;
How does it crashes in that case? I would have guess that write access
to ROM are ignored by default. Looking at other code, it seems they call
memory_region_set_readonly() instead of providing an accepts function.
Maybe readonly should be the default for a rom device?
The stack trace is:
0x000055555563a758 in memory_region_access_valid (mr=mr@entry=0x55555adb0d50,
addr=addr@entry=0, size=size@entry=1, is_write=is_write@entry=true) at
memory.c:1075
1075 if (!mr->ops->valid.unaligned && (addr & (size - 1))) {
(gdb) bt
#0 0x000055555563a758 in memory_region_access_valid
(mr=mr@entry=0x55555adb0d50, addr=addr@entry=0, size=size@entry=1,
is_write=is_write@entry=true) at memory.c:1075
#1 0x000055555563a968 in memory_region_dispatch_write (mr=0x55555adb0d50,
addr=0, data=82, size=1, attrs=...) at memory.c:1155
#2 0x00007fffe6516f35 in code_gen_buffer ()
#3 0x000055555560e4f3 in cpu_tb_exec (tb_ptr=0x7fffe6516ec0 <code_gen_buffer+8625856>
"A\213n\374\205\355\017\205\220", cpu=0x55555703f1c0) at cpu-exec.c:200
#4 cpu_mips_exec (cpu=cpu@entry=0x55555703f1c0) at cpu-exec.c:518
#5 0x000055555562aec6 in tcg_cpu_exec (cpu=0x55555703f1c0) at cpus.c:1402
#6 tcg_exec_all () at cpus.c:1434
#7 qemu_tcg_cpu_thread_fn (arg=<optimized out>) at cpus.c:1068
#8 0x00007ffff1dbd0a4 in start_thread (arg=0x7fffdf8f8700) at
pthread_create.c:309
#9 0x00007ffff1af204d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:111
With mr being the dp8393x prom.
I tested with memory_region_set_readonly() and a NULL operations, and the stack
trace is the same.
Only pflash devices use memory_region_init_rom_device. Other devices use
memory_region_init_ram + memory_region_set_readonly, which work.
Do you prefer the attached patch?
From bfe27278e08d1a1239ae674036114a21cb152582 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= <hpous...@reactos.org>
Date: Sun, 26 Jul 2015 22:32:55 +0200
Subject: [PATCH] net/dp8393x: do not use memory_region_init_rom_device with
NULL memory operations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Replace memory_region_init_rom_device() with memory_region_init_ram() and
memory_region_set_readonly().
This fixes a guest-triggerable QEMU crash when guest tries to write to PROM.
Signed-off-by: Hervé Poussineau <hpous...@reactos.org>
---
hw/net/dp8393x.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 8fafdb0..8ba1d0b 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -828,6 +828,7 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
dp8393xState *s = DP8393X(dev);
int i, checksum;
uint8_t *prom;
+ Error *local_err = NULL;
address_space_init(&s->as, s->dma_mr, "dp8393x");
memory_region_init_io(&s->mmio, OBJECT(dev), &dp8393x_ops, s,
@@ -840,8 +841,13 @@ static void dp8393x_realize(DeviceState *dev, Error **errp)
s->watchdog = timer_new_ns(QEMU_CLOCK_VIRTUAL, dp8393x_watchdog, s);
s->regs[SONIC_SR] = 0x0004; /* only revision recognized by Linux */
- memory_region_init_rom_device(&s->prom, OBJECT(dev), NULL, NULL,
- "dp8393x-prom", SONIC_PROM_SIZE, NULL);
+ memory_region_init_ram(&s->prom, OBJECT(dev),
+ "dp8393x-prom", SONIC_PROM_SIZE, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ memory_region_set_readonly(&s->prom, true);
prom = memory_region_get_ram_ptr(&s->prom);
checksum = 0;
for (i = 0; i < 6; i++) {
--
2.1.4
Hervé