On Fri, 2022-08-05 at 09:50 +0100, Peter Maydell wrote: > On Thu, 4 Aug 2022 at 19:50, Ilya Leoshkevich <i...@linux.ibm.com> > wrote: > > > > When the first instruction of a translation block is located in a > > non-readable page, qemu-user fills siginfo_t correctly. For the > > other > > instructions the result is as if it were the first instruction, > > which > > is not correct. > > > > The reason is that the current logic expects translate_insn() hook > > to > > stop at the page boundary. This way only the first instruction can > > cause a SEGV. However, this is quite difficult to properly > > implement > > when the problematic instruction crosses a page boundary, and > > indeed > > the actual implementations do not do this. Note that this can also > > break self-modifying code detection when only bytes on the second > > page > > are modified, but this is outside of the scope of this patch. > > Which guests do you observe this on ? I think we should indeed > fix this in the translators. More specifically, I think we should > get this correct already on Arm, and I would expect it to work > correctly on all the fixed-insn-width architectures, which can't > have page-crossing-insns to start with. x86 probably gets this wrong. > > thanks > -- PMM
I first discovered this on s390x, and then realized x86_64 is broken as well. Fixing this in translators means adding page boundary checks to all code loads. Actually, on s390x it doesn't look as nasty as I thought it would, since we quickly figure out the length and load everything in one place: $ grep ld.*code target/s390x/tcg/translate.c | wc -l 6 On x86_64 it's as bad as expected: $ grep x86_ld.*code target/i386/tcg/translate.c | wc -l 96 Implementing this there would mean changing x86_ldub_code() and friends to macros, and then making sure we undo everything that we did since the start of the instruction. E.g. bt/bts/btr/btc mix parsing and op emission. There might be something that touches DisasContext as well. Therefore I thought that the generic approach from this patch would be more reliable.
