| Message ID | 1336725610-8195-1-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On 11 May 2012 09:40, Alexander Graf <agraf@suse.de> wrote: > If we execute linux-user code that does the following: > > * A = mmap() > * execute code in A > * munmap(A) > * B = mmap(), but mmap returns the same address as A > * execute code in B > > we end up executing a stale cached tb that contains translated code > from A, while we want new code from B. > > This patch adds a TB flush for mmap'ed regions, before we return them, > avoiding the whole issue. It also adds a flush for munmap, so that we > don't execute stale TBs instead of getting a segfault. > > Reported-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Alexander Graf <agraf@suse.de> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
Ping? This is 1.1 material in my opinion... (patchwork url: http://patchwork.ozlabs.org/patch/158556/) -- PMM On 11 May 2012 17:25, Peter Maydell <peter.maydell@linaro.org> wrote: > On 11 May 2012 09:40, Alexander Graf <agraf@suse.de> wrote: >> If we execute linux-user code that does the following: >> >> * A = mmap() >> * execute code in A >> * munmap(A) >> * B = mmap(), but mmap returns the same address as A >> * execute code in B >> >> we end up executing a stale cached tb that contains translated code >> from A, while we want new code from B. >> >> This patch adds a TB flush for mmap'ed regions, before we return them, >> avoiding the whole issue. It also adds a flush for munmap, so that we >> don't execute stale TBs instead of getting a segfault. >> >> Reported-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Alexander Graf <agraf@suse.de> > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > -- PMM
Riku, Can you review/ack this patch? Regards, Anthony Liguori On 05/15/2012 03:35 PM, Peter Maydell wrote: > Ping? This is 1.1 material in my opinion... > > (patchwork url: http://patchwork.ozlabs.org/patch/158556/) > > -- PMM > > On 11 May 2012 17:25, Peter Maydell<peter.maydell@linaro.org> wrote: >> On 11 May 2012 09:40, Alexander Graf<agraf@suse.de> wrote: >>> If we execute linux-user code that does the following: >>> >>> * A = mmap() >>> * execute code in A >>> * munmap(A) >>> * B = mmap(), but mmap returns the same address as A >>> * execute code in B >>> >>> we end up executing a stale cached tb that contains translated code >>> from A, while we want new code from B. >>> >>> This patch adds a TB flush for mmap'ed regions, before we return them, >>> avoiding the whole issue. It also adds a flush for munmap, so that we >>> don't execute stale TBs instead of getting a segfault. >>> >>> Reported-by: Peter Maydell<peter.maydell@linaro.org> >>> Signed-off-by: Alexander Graf<agraf@suse.de> >> >> Reviewed-by: Peter Maydell<peter.maydell@linaro.org> >> >> -- PMM >
On Tue, May 15, 2012 at 04:32:51PM -0500, Anthony Liguori wrote: > Riku, > > Can you review/ack this patch? Acked-by: Riku Voipio <riku.voipio@linaro.org> Are there any other Linux-user patches to consider for 1.1 ? > > Regards, > > Anthony Liguori > > > On 05/15/2012 03:35 PM, Peter Maydell wrote: > >Ping? This is 1.1 material in my opinion... > > > >(patchwork url: http://patchwork.ozlabs.org/patch/158556/) > > > >-- PMM > > > >On 11 May 2012 17:25, Peter Maydell<peter.maydell@linaro.org> wrote: > >>On 11 May 2012 09:40, Alexander Graf<agraf@suse.de> wrote: > >>>If we execute linux-user code that does the following: > >>> > >>> * A = mmap() > >>> * execute code in A > >>> * munmap(A) > >>> * B = mmap(), but mmap returns the same address as A > >>> * execute code in B > >>> > >>>we end up executing a stale cached tb that contains translated code > >>>from A, while we want new code from B. > >>> > >>>This patch adds a TB flush for mmap'ed regions, before we return them, > >>>avoiding the whole issue. It also adds a flush for munmap, so that we > >>>don't execute stale TBs instead of getting a segfault. > >>> > >>>Reported-by: Peter Maydell<peter.maydell@linaro.org> > >>>Signed-off-by: Alexander Graf<agraf@suse.de> > >> > >>Reviewed-by: Peter Maydell<peter.maydell@linaro.org> > >> > >>-- PMM > >
Am 16.05.2012 11:26, schrieb Riku Voipio: > On Tue, May 15, 2012 at 04:32:51PM -0500, Anthony Liguori wrote: >> Riku, >> >> Can you review/ack this patch? > > Acked-by: Riku Voipio <riku.voipio@linaro.org> > > Are there any other Linux-user patches to consider for 1.1 ? I had noticed that Alex' binfmt wrappers did not get applied, but although they fix argv[0] issues they're more of a feature than a bugfix. Andreas
diff --git a/exec-all.h b/exec-all.h index c1b7e1f..9bda7f7 100644 --- a/exec-all.h +++ b/exec-all.h @@ -96,6 +96,8 @@ void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1); int page_unprotect(target_ulong address, uintptr_t pc, void *puc); void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end, int is_cpu_write_access); +void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end, + int is_cpu_write_access); #if !defined(CONFIG_USER_ONLY) /* cputlb.c */ void tlb_flush_page(CPUArchState *env, target_ulong addr); diff --git a/exec.c b/exec.c index 0607c9b..a0494c7 100644 --- a/exec.c +++ b/exec.c @@ -1075,6 +1075,23 @@ TranslationBlock *tb_gen_code(CPUArchState *env, return tb; } +/* + * invalidate all TBs which intersect with the target physical pages + * starting in range [start;end[. NOTE: start and end may refer to + * different physical pages. 'is_cpu_write_access' should be true if called + * from a real cpu write access: the virtual CPU will exit the current + * TB if code is modified inside this TB. + */ +void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end, + int is_cpu_write_access) +{ + while (start < end) { + tb_invalidate_phys_page_range(start, end, is_cpu_write_access); + start &= TARGET_PAGE_MASK; + start += TARGET_PAGE_SIZE; + } +} + /* invalidate all TBs which intersect with the target physical page starting in range [start;end[. NOTE: start and end must refer to the same physical page. 'is_cpu_write_access' should be true if called diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 7125d1c..d9468fe 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -573,6 +573,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, page_dump(stdout); printf("\n"); #endif + tb_invalidate_phys_range(start, start + len, 0); mmap_unlock(); return start; fail: @@ -675,8 +676,10 @@ int target_munmap(abi_ulong start, abi_ulong len) } } - if (ret == 0) + if (ret == 0) { page_set_flags(start, start + len, 0); + tb_invalidate_phys_range(start, start + len, 0); + } mmap_unlock(); return ret; } @@ -754,6 +757,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size, page_set_flags(old_addr, old_addr + old_size, 0); page_set_flags(new_addr, new_addr + new_size, prot | PAGE_VALID); } + tb_invalidate_phys_range(new_addr, new_addr + new_size, 0); mmap_unlock(); return new_addr; }
If we execute linux-user code that does the following: * A = mmap() * execute code in A * munmap(A) * B = mmap(), but mmap returns the same address as A * execute code in B we end up executing a stale cached tb that contains translated code from A, while we want new code from B. This patch adds a TB flush for mmap'ed regions, before we return them, avoiding the whole issue. It also adds a flush for munmap, so that we don't execute stale TBs instead of getting a segfault. Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Alexander Graf <agraf@suse.de> --- v1 -> v2: - rebase on current git - fix munmap too --- exec-all.h | 2 ++ exec.c | 17 +++++++++++++++++ linux-user/mmap.c | 6 +++++- 3 files changed, 24 insertions(+), 1 deletions(-)