[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] Fix unassigned memory access handling
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH v2] Fix unassigned memory access handling |
Date: |
Mon, 1 Aug 2011 20:23:38 +0000 |
On Mon, Aug 1, 2011 at 4:26 PM, Bob Breuer <address@hidden> wrote:
> Blue Swirl wrote:
>> cea5f9a28faa528b6b1b117c9ab2d8828f473fef exposed bugs in unassigned memory
>> access handling. Fix them by always passing CPUState to the handlers.
>>
>> Reported-by: Hervé Poussineau <address@hidden>
>> Signed-off-by: Blue Swirl <address@hidden>
>> ---
>> v2: don't try to restore env since all targets eventually always call
>> cpu_loop_exit() which will not return.
>>
>> exec-all.h | 2 +-
>> exec.c | 12 ++++++------
>> target-alpha/cpu.h | 5 +++--
>> target-alpha/op_helper.c | 6 ++++--
>> target-microblaze/cpu.h | 4 ++--
>> target-microblaze/op_helper.c | 14 ++++----------
>> target-mips/cpu.h | 4 ++--
>> target-mips/op_helper.c | 6 ++++--
>> target-sparc/cpu.h | 4 ++--
>> target-sparc/op_helper.c | 26 ++++++++++++++++++++------
>> 10 files changed, 48 insertions(+), 35 deletions(-)
>>
>> diff --git a/exec-all.h b/exec-all.h
>> index 69acf3b..9b8d62c 100644
>> --- a/exec-all.h
>> +++ b/exec-all.h
>> @@ -323,7 +323,7 @@ static inline tb_page_addr_t
>> get_page_addr_code(CPUState *env1, target_ulong add
>> pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK;
>> if (pd > IO_MEM_ROM && !(pd & IO_MEM_ROMD)) {
>> #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
>> - do_unassigned_access(addr, 0, 1, 0, 4);
>> + cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
>> #else
>> cpu_abort(env1, "Trying to execute code outside RAM or ROM at
>> 0x" TARGET_FMT_lx "\n", addr);
>> #endif
>> diff --git a/exec.c b/exec.c
>> index f1777e6..16e16f3 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3236,7 +3236,7 @@ static uint32_t unassigned_mem_readb(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 1);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 1);
>> #endif
>> return 0;
>> }
>> @@ -3247,7 +3247,7 @@ static uint32_t unassigned_mem_readw(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 2);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 2);
>> #endif
>> return 0;
>> }
>> @@ -3258,7 +3258,7 @@ static uint32_t unassigned_mem_readl(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 4);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 4);
>> #endif
>> return 0;
>> }
>> @@ -3269,7 +3269,7 @@ static void unassigned_mem_writeb(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 1);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 1);
>> #endif
>> }
>>
>> @@ -3279,7 +3279,7 @@ static void unassigned_mem_writew(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 2);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 2);
>> #endif
>> }
>>
>> @@ -3289,7 +3289,7 @@ static void unassigned_mem_writel(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 4);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 4);
>> #endif
>> }
>>
>
> [..snip..]
>
>> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
>> index 4edae78..f6cb300 100644
>> --- a/target-sparc/cpu.h
>> +++ b/target-sparc/cpu.h
>> @@ -510,8 +510,8 @@ static inline int tlb_compare_context(const
>> SparcTLBEntry *tlb,
>>
>> /* cpu-exec.c */
>> #if !defined(CONFIG_USER_ONLY)
>> -void do_unassigned_access(target_phys_addr_t addr, int is_write, int
>> is_exec,
>> - int is_asi, int size);
>> +void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
>> + int is_write, int is_exec, int is_asi, int size);
>> target_phys_addr_t cpu_get_phys_page_nofault(CPUState *env, target_ulong
>> addr,
>> int mmu_idx);
>>
>> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
>> index fd0cfbd..101edfb 100644
>> --- a/target-sparc/op_helper.c
>> +++ b/target-sparc/op_helper.c
>> @@ -79,9 +79,14 @@
>> #define CACHE_CTRL_FD (1 << 22) /* Flush Data cache (Write only) */
>> #define CACHE_CTRL_DS (1 << 23) /* Data cache snoop enable */
>>
>> -#if defined(CONFIG_USER_ONLY) && defined(TARGET_SPARC64)
>> +#if !defined(CONFIG_USER_ONLY)
>> +static void do_unassigned_access(target_phys_addr_t addr, int is_write,
>> + int is_exec, int is_asi, int size);
>> +#else
>> +#ifdef TARGET_SPARC64
>> static void do_unassigned_access(target_ulong addr, int is_write, int
>> is_exec,
>> - int is_asi, int size);
>> + int is_asi, int size);
>> +#endif
>> #endif
>>
>> #if defined(TARGET_SPARC64) && !defined(CONFIG_USER_ONLY)
>> @@ -4206,8 +4211,8 @@ void tlb_fill(target_ulong addr, int is_write,
>> int mmu_idx, void *retaddr)
>>
>> #ifndef TARGET_SPARC64
>> #if !defined(CONFIG_USER_ONLY)
>> -void do_unassigned_access(target_phys_addr_t addr, int is_write, int
>> is_exec,
>> - int is_asi, int size)
>> +static void do_unassigned_access(target_phys_addr_t addr, int is_write,
>> + int is_exec, int is_asi, int size)
>> {
>> CPUState *saved_env;
>> int fault_type;
>> @@ -4272,8 +4277,8 @@ void do_unassigned_access(target_phys_addr_t
>> addr, int is_write, int is_exec,
>> static void do_unassigned_access(target_ulong addr, int is_write, int
>> is_exec,
>> int is_asi, int size)
>> #else
>> -void do_unassigned_access(target_phys_addr_t addr, int is_write, int
>> is_exec,
>> - int is_asi, int size)
>> +static void do_unassigned_access(target_phys_addr_t addr, int is_write,
>> + int is_exec, int is_asi, int size)
>> #endif
>> {
>> CPUState *saved_env;
>> @@ -4322,3 +4327,12 @@ void helper_tick_set_limit(void *opaque, uint64_t
>> limit)
>> #endif
>> }
>> #endif
>> +
>> +#if !defined(CONFIG_USER_ONLY)
>> +void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
>> + int is_write, int is_exec, int is_asi, int size)
>> +{
>> + env = env1;
>> + do_unassigned_access(addr, is_write, is_exec, is_asi, size);
>> +}
>> +#endif
>>
>
> Blue,
>
> After this patch, I can trigger stack corruption for sparc32 unassigned
> memory. In OpenBIOS, try dumping memory beyond what's been allocated,
> i.e. with 64M of guest memory, dump at address 0x4000000. I get a
> similar failure with the SS-20 OBP during it's memory size probe.
>
> Here's a run under gdb (32-bit x86 host):
>
> address@hidden:~/qemu$ gdb --args
> ./build-debian/sparc-softmmu/qemu-system-sparc -bios
> ./qemu-git/pc-bios/openbios-sparc32 -m 64 -nographic
> GNU gdb (GDB) 7.0.1-debian
> Copyright (C) 2009 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law. Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "i486-linux-gnu".
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>...
> Reading symbols from
> /home/bob/qemu/build-debian/sparc-softmmu/qemu-system-sparc...done.
> (gdb) break cpu_unassigned_access
> Breakpoint 1 at 0x8151be0: file
> /home/bob/qemu/qemu-git/target-sparc/op_helper.c, line 4372.
> (gdb) run
> Starting program:
> /home/bob/qemu/build-debian/sparc-softmmu/qemu-system-sparc -bios
> ./qemu-git/pc-bios/openbios-sparc32 -m 64 -nographic
> [Thread debugging using libthread_db enabled]
> Configuration device id QEMU version 1 machine id 32
> CPUs: 1 x FMI,MB86904
> UUID: 00000000-0000-0000-0000-000000000000
> Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:16
> Type 'help' for detailed information
> Trying disk...
> No valid state has been set by load or init-program
>
> 0 > 4000000 10 dump
> 4000000 U
> Breakpoint 1, cpu_unassigned_access (env1=0x87eb618, addr=68468867078,
> is_write=1, is_exec=0, is_asi=0, size=1)
> at /home/bob/qemu/qemu-git/target-sparc/op_helper.c:4372
> 4372 {
> (gdb) n
> 4373 env = env1;
> (gdb) print env
> $1 = (struct CPUSPARCState *) 0xbffff088
> (gdb) print env1
> $2 = (struct CPUSPARCState *) 0x87eb618
> (gdb) bt
> #0 cpu_unassigned_access (env1=0x87eb618, addr=68468867078, is_write=1,
> is_exec=0, is_asi=0, size=1)
> at /home/bob/qemu/qemu-git/target-sparc/op_helper.c:4373
> #1 0x0811502c in unassigned_mem_writeb (opaque=0x0, addr=68468867078,
> val=85)
> at /home/bob/qemu/qemu-git/exec.c:3282
> #2 0x081169f2 in cpu_physical_memory_rw (addr=68468867078,
> buf=0xbffff14b "U", len=1, is_write=1)
> at /home/bob/qemu/qemu-git/exec.c:3917
> #3 0x08117000 in cpu_physical_memory_write (addr=68468867078, val=85)
> at /home/bob/qemu/qemu-git/cpu-common.h:96
> #4 stb_phys (addr=68468867078, val=85) at
> /home/bob/qemu/qemu-git/exec.c:4513
> #5 0xb69479e4 in ?? ()
> Backtrace stopped: previous frame inner to this frame (corrupt stack?)
> (gdb) break abort
> Breakpoint 2 at 0xb7b477a5
> (gdb) c
> Continuing.
> *** stack smashing detected ***:
> /home/bob/qemu/build-debian/sparc-softmmu/qemu-system-sparc terminated
> ======= Backtrace: =========
> /lib/libc.so.6(__fortify_fail+0x40)[0xb7bfa1f0]
> /lib/libc.so.6(+0xe01aa)[0xb7bfa1aa]
> /home/bob/qemu/build-debian/sparc-softmmu/qemu-system-sparc[0x8115045]
> ======= Memory map: ========
> 08048000-081d0000 r-xp 00000000 08:01 812127
> /home/bob/qemu/build-debian/sparc-softmmu/qemu-system-sparc
> 081d0000-081da000 rw-p 00188000 08:01 812127
> /home/bob/qemu/build-debian/sparc-softmmu/qemu-system-sparc
> 081da000-087d2000 rw-p 00000000 00:00 0
> 087d2000-087d4000 rwxp 00000000 00:00 0
> 087d4000-0890c000 rw-p 00000000 00:00 0
>
> [..snip..]
>
> bffeb000-c0000000 rwxp 00000000 00:00 0 [stack]
>
> Breakpoint 2, 0xb7b477a5 in abort () from /lib/libc.so.6
> (gdb) bt
> #0 0xb7b477a5 in abort () from /lib/libc.so.6
> #1 0xb7b7afbd in ?? () from /lib/libc.so.6
> #2 0xb7bfa1f0 in __fortify_fail () from /lib/libc.so.6
> #3 0xb7bfa1aa in __stack_chk_fail () from /lib/libc.so.6
> #4 0x08115045 in unassigned_mem_writeb (opaque=0x6000,
> addr=18436631178074153520, val=0) at /home/bob/qemu/qemu-git/exec.c:3284
> Backtrace stopped: previous frame inner to this frame (corrupt stack?)
> (gdb)
It doesn't happen on x86_64 host. Otherwise this is very puzzling.