| Message ID | 20220823220542.1993395-6-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: Fix siginfo_t contents when jumping to non-readable pages | expand |
On Tue, 2022-08-23 at 15:05 -0700, Richard Henderson wrote: > From: Ilya Leoshkevich <iii@linux.ibm.com> > > Currently it's possible to execute pages that do not have PAGE_EXEC > if there is an existing translation block. Fix by clearing > tb_jmp_cache > and invalidating TBs, which forces recheck of permission bits. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > Message-Id: <20220817150506.592862-2-iii@linux.ibm.com> > [rth: Invalidate is required -- e.g. riscv fallthrough cross test] > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > fixup mprotect > --- > linux-user/mmap.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index 048c4135af..e9dc8848be 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -115,6 +115,7 @@ int target_mprotect(abi_ulong start, abi_ulong > len, int target_prot) > { > abi_ulong end, host_start, host_end, addr; > int prot1, ret, page_flags, host_prot; > + CPUState *cpu; > > trace_target_mprotect(start, len, target_prot); > > @@ -177,7 +178,14 @@ int target_mprotect(abi_ulong start, abi_ulong > len, int target_prot) > goto error; > } > } > + > page_set_flags(start, start + len, page_flags); > + tb_invalidate_phys_range(start, start + len); > + > + CPU_FOREACH(cpu) { > + cpu_tb_jmp_cache_clear(cpu); > + } > + > mmap_unlock(); > return 0; > error: I think adding tb_invalidate_phys_range() obviates the need for cpu_tb_jmp_cache_clear()? The lookup may still find an invalidated tb, but it will have CF_INVALID set. The following worked for me: diff --git a/linux-user/mmap.c b/linux-user/mmap.c index e9dc8848bed..b58e3eeb198 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -115,7 +115,6 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot) { abi_ulong end, host_start, host_end, addr; int prot1, ret, page_flags, host_prot; - CPUState *cpu; trace_target_mprotect(start, len, target_prot); @@ -182,10 +181,6 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot) page_set_flags(start, start + len, page_flags); tb_invalidate_phys_range(start, start + len); - CPU_FOREACH(cpu) { - cpu_tb_jmp_cache_clear(cpu); - } - mmap_unlock(); return 0; error:
On 8/31/22 00:17, Ilya Leoshkevich wrote: >> page_set_flags(start, start + len, page_flags); >> + tb_invalidate_phys_range(start, start + len); >> + >> + CPU_FOREACH(cpu) { >> + cpu_tb_jmp_cache_clear(cpu); >> + } >> + >> mmap_unlock(); >> return 0; >> error: > > I think adding tb_invalidate_phys_range() obviates the need for > cpu_tb_jmp_cache_clear()? The lookup may still find an invalidated tb, > but it will have CF_INVALID set. Quite right. And we definitely don't want to have to touch a list of all threads if its not necessary. r~
diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 048c4135af..e9dc8848be 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -115,6 +115,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot) { abi_ulong end, host_start, host_end, addr; int prot1, ret, page_flags, host_prot; + CPUState *cpu; trace_target_mprotect(start, len, target_prot); @@ -177,7 +178,14 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot) goto error; } } + page_set_flags(start, start + len, page_flags); + tb_invalidate_phys_range(start, start + len); + + CPU_FOREACH(cpu) { + cpu_tb_jmp_cache_clear(cpu); + } + mmap_unlock(); return 0; error: