Hi Philippe! On 4/23/19 3:48 PM, Philippe Mathieu-Daudé wrote: > Hi Jakub, > > On 4/23/19 1:00 PM, Jakub Jermář wrote: >> This commit addresses QEMU Bug #1825311: >> >> mips_cpu_handle_mmu_fault renders all accessed pages executable >> >> It allows finer-grained control over whether the accessed page should be >> executable by moving the decision to the underlying map_address >> function, which has more information for this. >> >> As a result, pages that have the XI bit set in the TLB and are accessed >> for read/write, don't suddenly end up being executable. >> > > Fixes: https://bugs.launchpad.net/qemu/+bug/1825311 > >> Signed-off-by: Jakub Jermář <jakub.jer...@kernkonzept.com> >> --- >> target/mips/helper.c | 17 ++++++++++------- >> 1 file changed, 10 insertions(+), 7 deletions(-) >> >> diff --git a/target/mips/helper.c b/target/mips/helper.c >> index c44cdca3b5..132d073fbe 100644 >> --- a/target/mips/helper.c >> +++ b/target/mips/helper.c >> @@ -43,7 +43,7 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr >> *physical, int *prot, >> target_ulong address, int rw, int access_type) >> { >> *physical = address; >> - *prot = PAGE_READ | PAGE_WRITE; >> + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; >> return TLBRET_MATCH; >> } >> >> @@ -61,7 +61,7 @@ int fixed_mmu_map_address (CPUMIPSState *env, hwaddr >> *physical, int *prot, >> else >> *physical = address; >> >> - *prot = PAGE_READ | PAGE_WRITE; >> + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; >> return TLBRET_MATCH; >> } >> >> @@ -101,6 +101,9 @@ int r4k_map_address (CPUMIPSState *env, hwaddr >> *physical, int *prot, >> *prot = PAGE_READ; >> if (n ? tlb->D1 : tlb->D0) >> *prot |= PAGE_WRITE; >> + if (!(n ? tlb->XI1 : tlb->XI0)) { >> + *prot |= PAGE_EXEC; >> + } > > This was indeed missed in commit 2fb58b73746e. > >> return TLBRET_MATCH; >> } >> return TLBRET_DIRTY; >> @@ -182,7 +185,7 @@ static int get_seg_physical_address(CPUMIPSState *env, >> hwaddr *physical, >> } else { >> /* The segment is unmapped */ >> *physical = physical_base | (real_address & segmask); >> - *prot = PAGE_READ | PAGE_WRITE; >> + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; >> return TLBRET_MATCH; >> } >> } >> @@ -913,8 +916,8 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr >> address, int size, int rw, >> } >> if (ret == TLBRET_MATCH) { >> tlb_set_page(cs, address & TARGET_PAGE_MASK, >> - physical & TARGET_PAGE_MASK, prot | PAGE_EXEC, >> - mmu_idx, TARGET_PAGE_SIZE); >> + physical & TARGET_PAGE_MASK, prot, mmu_idx, >> + TARGET_PAGE_SIZE); >> ret = 0; >> } else if (ret < 0) >> #endif >> @@ -936,8 +939,8 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr >> address, int size, int rw, >> address, rw, access_type, >> mmu_idx); >> if (ret == TLBRET_MATCH) { >> tlb_set_page(cs, address & TARGET_PAGE_MASK, >> - physical & TARGET_PAGE_MASK, prot | PAGE_EXEC, >> - mmu_idx, TARGET_PAGE_SIZE); >> + physical & TARGET_PAGE_MASK, prot, mmu_idx, >> + TARGET_PAGE_SIZE); >> ret = 0; >> return ret; >> } >> > > Your patch looks correct, but I'd like to test it. > Do you have a reproducer? > Can you describe the command line you used?
I've just attached a reproducer image and script to the bug. It's a 32-bit little-endian test binary running on top of the L4Re microkernel. Let me know if you also need a 64-bit version. I tested both 32 and 64-bit versions of the reproducer and also checked to see that the the other images I have lying around here (Linux 2.6.32 big endian and HelenOS master little-endian, both 32-bit for 4Kc) continue to run without regressions. Best regards, Jakub > > Thanks, > > Phil. > -- Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth