| Message ID | 20220817150506.592862-1-iii@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | linux-user: Fix siginfo_t contents when jumping to non-readable pages | expand |
On 8/17/22 10:05, Ilya Leoshkevich wrote: > Hi, > > I noticed that when we get a SEGV due to jumping to non-readable > memory, sometimes si_addr and program counter in siginfo_t are slightly > off. I tracked this down to the assumption that translators stop before > the end of a page, while in reality they may stop right after it. > > Patch 1 fixes an invalidation issue, which may prevent SEGV from > happening altogether. > Patches 2-3 fix the main issue on x86_64 and s390x. Many other > architectures have fixed-size instructions and are not affected. > Patch 4 adds tests. > > Note: this series depends on [1]. Hah. I was just thinking that I should queue your patch set to tcg-next-7.2, and then rebase my stuff off of that. It would ensure that I have your test cases in tree so that I don't keep regressing them on you. :-) I'll cherry pick the one patch you're depending on. r~
On Wed, 2022-08-17 at 11:23 -0500, Richard Henderson wrote: > On 8/17/22 10:05, Ilya Leoshkevich wrote: > > Hi, > > > > I noticed that when we get a SEGV due to jumping to non-readable > > memory, sometimes si_addr and program counter in siginfo_t are > > slightly > > off. I tracked this down to the assumption that translators stop > > before > > the end of a page, while in reality they may stop right after it. > > > > Patch 1 fixes an invalidation issue, which may prevent SEGV from > > happening altogether. > > Patches 2-3 fix the main issue on x86_64 and s390x. Many other > > architectures have fixed-size instructions and are not affected. > > Patch 4 adds tests. > > > > Note: this series depends on [1]. > > Hah. I was just thinking that I should queue your patch set to tcg- > next-7.2, and then > rebase my stuff off of that. It would ensure that I have your test > cases in tree so that > I don't keep regressing them on you. :-) > > I'll cherry pick the one patch you're depending on. > > > r~ I just checked and cherry-picking [1] and [2] before this series should be enough. [1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02462.html [2] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02461.html
On 8/17/22 23:05, Ilya Leoshkevich wrote: > Hi, > > I noticed that when we get a SEGV due to jumping to non-readable > memory, sometimes si_addr and program counter in siginfo_t are slightly > off. I tracked this down to the assumption that translators stop before > the end of a page, while in reality they may stop right after it. Hi, Could this be related to issue 1155 [1]? On RISC-V, I'm getting incorrect [m|s]tval/[m|s]epc combinations for page faults in system emulation and incorrect si_addr and program counter on SIGSEGV in user emulation. Since it seems to only affect instructions that cross page boundaries, and RISC-V also has variable length instructions, it seems that I've run into the same problem as what is fixed here. Could this fix be extended be extended to targets/riscv? dram [1]: https://gitlab.com/qemu-project/qemu/-/issues/1155 > Patch 1 fixes an invalidation issue, which may prevent SEGV from > happening altogether. > Patches 2-3 fix the main issue on x86_64 and s390x. Many other > architectures have fixed-size instructions and are not affected. > Patch 4 adds tests. > > Note: this series depends on [1]. > > Best regards, > Ilya > > v1: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00822.html > v1 -> v2: Fix individual translators instead of translator_loop > (Peter). > > v2: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01079.html > v2 -> v3: Peek at the next instruction on s390x (Richard). > Undo more on i386 (Richard). > Check PAGE_EXEC, not PAGE_READ (Peter, Richard). > > v3: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01306.html > v3 -> v4: Improve the commit message in patch 1 to better reflect what > exactly is being fixed there. > Factor out the is_same_page() patch (Richard). > Do not touch the common code in the i386 fix (Richard). > > v4: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01747.html > v4 -> v5: Drop patch 2. > Use a different fix for the invalidation issue based on > discussion with Richard [2]. > > [1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02472.html > [2] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02556.html > > Ilya Leoshkevich (4): > linux-user: Clear tb_jmp_cache on mprotect() > target/s390x: Make translator stop before the end of a page > target/i386: Make translator stop before the end of a page > tests/tcg: Test siginfo_t contents when jumping to non-readable pages > > linux-user/mmap.c | 14 +++ > target/i386/tcg/translate.c | 25 +++++- > target/s390x/tcg/translate.c | 15 +++- > tests/tcg/multiarch/noexec.h | 114 ++++++++++++++++++++++++ > tests/tcg/s390x/Makefile.target | 1 + > tests/tcg/s390x/noexec.c | 145 +++++++++++++++++++++++++++++++ > tests/tcg/x86_64/Makefile.target | 3 +- > tests/tcg/x86_64/noexec.c | 116 +++++++++++++++++++++++++ > 8 files changed, 427 insertions(+), 6 deletions(-) > create mode 100644 tests/tcg/multiarch/noexec.h > create mode 100644 tests/tcg/s390x/noexec.c > create mode 100644 tests/tcg/x86_64/noexec.c >
On Fri, 2022-08-19 at 00:55 +0800, Vivian Wang wrote: > Hi, > Could this be related to issue 1155 [1]? On RISC-V, I'm getting > incorrect [m|s]tval/[m|s]epc combinations for page faults in system > emulation and incorrect si_addr and program counter on SIGSEGV in > user emulation. Since it seems to only affect instructions that cross > page boundaries, and RISC-V also has variable length instructions, it > seems that I've run into the same problem as what is fixed here. > Could this fix be extended be extended to targets/riscv? > dram > [1]: https://gitlab.com/qemu-project/qemu/-/issues/1155 Yes, this looks quite similar. I'm not too familiar with riscv, but I just googled [1]. If the following is correct: --- However, the instruction set reserves enough opcode space to make it possible to differentiate between 16-bit, 32-bit, 48-bit, and 64-bit instructions. Instructions that start with binary 11 (in the lowest bit position of the instruction) are 32-bit sized instructions (but one pattern is reserved: so they cannot start with 11111). The compact instructions use 00, 01, and 10 in that same position. 48-bit instructions use starting sequence 011111, and 64-bit instructions start with 0111111. --- then we can fix this the same way s390x is being fixed here. [1] https://stackoverflow.com/questions/56874101/how-does-risc-v-variable-length-of-instruction-work-in-detail
On 8/18/22 09:55, Vivian Wang wrote: > On 8/17/22 23:05, Ilya Leoshkevich wrote: >> Hi, >> >> I noticed that when we get a SEGV due to jumping to non-readable >> memory, sometimes si_addr and program counter in siginfo_t are slightly >> off. I tracked this down to the assumption that translators stop before >> the end of a page, while in reality they may stop right after it. > > Hi, > > Could this be related to issue 1155 [1]? On RISC-V, I'm getting incorrect > [m|s]tval/[m|s]epc combinations for page faults in system emulation and incorrect si_addr > and program counter on SIGSEGV in user emulation. Since it seems to only affect > instructions that cross page boundaries, and RISC-V also has variable length instructions, > it seems that I've run into the same problem as what is fixed here. It seems likely, and the code at the end of riscv_tr_translate_insn is wrong. > Could this fix be extended be extended to targets/riscv? I'll write up something. r~
Hi, I noticed that when we get a SEGV due to jumping to non-readable memory, sometimes si_addr and program counter in siginfo_t are slightly off. I tracked this down to the assumption that translators stop before the end of a page, while in reality they may stop right after it. Patch 1 fixes an invalidation issue, which may prevent SEGV from happening altogether. Patches 2-3 fix the main issue on x86_64 and s390x. Many other architectures have fixed-size instructions and are not affected. Patch 4 adds tests. Note: this series depends on [1]. Best regards, Ilya v1: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00822.html v1 -> v2: Fix individual translators instead of translator_loop (Peter). v2: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01079.html v2 -> v3: Peek at the next instruction on s390x (Richard). Undo more on i386 (Richard). Check PAGE_EXEC, not PAGE_READ (Peter, Richard). v3: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01306.html v3 -> v4: Improve the commit message in patch 1 to better reflect what exactly is being fixed there. Factor out the is_same_page() patch (Richard). Do not touch the common code in the i386 fix (Richard). v4: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01747.html v4 -> v5: Drop patch 2. Use a different fix for the invalidation issue based on discussion with Richard [2]. [1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02472.html [2] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02556.html Ilya Leoshkevich (4): linux-user: Clear tb_jmp_cache on mprotect() target/s390x: Make translator stop before the end of a page target/i386: Make translator stop before the end of a page tests/tcg: Test siginfo_t contents when jumping to non-readable pages linux-user/mmap.c | 14 +++ target/i386/tcg/translate.c | 25 +++++- target/s390x/tcg/translate.c | 15 +++- tests/tcg/multiarch/noexec.h | 114 ++++++++++++++++++++++++ tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/noexec.c | 145 +++++++++++++++++++++++++++++++ tests/tcg/x86_64/Makefile.target | 3 +- tests/tcg/x86_64/noexec.c | 116 +++++++++++++++++++++++++ 8 files changed, 427 insertions(+), 6 deletions(-) create mode 100644 tests/tcg/multiarch/noexec.h create mode 100644 tests/tcg/s390x/noexec.c create mode 100644 tests/tcg/x86_64/noexec.c