| Message ID | 1488987550-21843-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 08.03.17 at 16:39, <andrew.cooper3@citrix.com> wrote: > ...rather than leaving fragments of old instructions in place. This reduces > the chances of something going further-wrong (as the debug trap will be cause > and terminate the guest) in a cascade-failure where we end up executing the > instruction fragments. Where are you taking the "and terminate the guest" from? As far as I can see, do_int3() does nothing at all for an INT3 from hypervisor context (without CRASH_DEBUG), so we'd just take a row of INT3s until we hit the end of stub space (running either past the page boundary or into the next CPU's stub space, which is the syscall entry code). > Before: > (XEN) d2v0 exception 6 (ec=0000) in emulation stub (line 6239) > (XEN) d2v0 stub: c4 e1 44 77 c3 80 d0 82 ff ff ff d1 90 ec 90 Hmm, this is concerning: I don't think we have ways to generate 15-byte instructions into the stub, so where are all these non-zero bytes coming from? After all alloc_stub_page() pre-fills the page with all 0xCC. > After: > (XEN) d3v0 exception 6 (ec=0000) in emulation stub (line 6239) > (XEN) d3v0 stub: c4 e1 44 77 c3 cc cc cc cc cc cc cc cc cc cc > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > > Semi-RFC: I really don't like (ab)use of memset, but can't think of a cleaner > way of doing this. What abuse are you seeing here? Jan
On 08/03/17 16:23, Jan Beulich wrote: >>>> On 08.03.17 at 16:39, <andrew.cooper3@citrix.com> wrote: >> ...rather than leaving fragments of old instructions in place. This reduces >> the chances of something going further-wrong (as the debug trap will be cause >> and terminate the guest) in a cascade-failure where we end up executing the >> instruction fragments. > Where are you taking the "and terminate the guest" from? Memory, which has proved to be a poor source of information. > As far as > I can see, do_int3() does nothing at all for an INT3 from hypervisor > context (without CRASH_DEBUG), so we'd just take a row of INT3s > until we hit the end of stub space (running either past the page > boundary or into the next CPU's stub space, which is the syscall > entry code). I will see about fixing this, so any exception in the stubs gets bounced back. > >> Before: >> (XEN) d2v0 exception 6 (ec=0000) in emulation stub (line 6239) >> (XEN) d2v0 stub: c4 e1 44 77 c3 80 d0 82 ff ff ff d1 90 ec 90 > Hmm, this is concerning: I don't think we have ways to generate > 15-byte instructions into the stub, so where are all these non-zero > bytes coming from? This is the host_to_guest_gpr_switch invocation in io_emul_stub_setup(), which uses 15 bytes of stub. > After all alloc_stub_page() pre-fills the page > with all 0xCC. > >> After: >> (XEN) d3v0 exception 6 (ec=0000) in emulation stub (line 6239) >> (XEN) d3v0 stub: c4 e1 44 77 c3 cc cc cc cc cc cc cc cc cc cc >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> >> Semi-RFC: I really don't like (ab)use of memset, but can't think of a cleaner >> way of doing this. > What abuse are you seeing here? The semantics used here are "return memset((a = b) + offset, ...);" where the assignment is unrelated to the value returned, which is propagated through memset returning its first parameter. ~Andrew
diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c index 51df340..cc334ca 100644 --- a/xen/arch/x86/x86_emulate.c +++ b/xen/arch/x86/x86_emulate.c @@ -30,8 +30,8 @@ BUILD_BUG_ON(STUB_BUF_SIZE / 2 < MAX_INST_LEN + 1); \ ASSERT(!(stb).ptr); \ (stb).addr = this_cpu(stubs.addr) + STUB_BUF_SIZE / 2; \ - ((stb).ptr = map_domain_page(_mfn(this_cpu(stubs.mfn)))) + \ - ((stb).addr & ~PAGE_MASK); \ + memset(((stb).ptr = map_domain_page(_mfn(this_cpu(stubs.mfn)))) + \ + ((stb).addr & ~PAGE_MASK), 0xcc, STUB_BUF_SIZE / 2); \ }) #define put_stub(stb) ({ \ if ( (stb).ptr ) \
...rather than leaving fragments of old instructions in place. This reduces the chances of something going further-wrong (as the debug trap will be cause and terminate the guest) in a cascade-failure where we end up executing the instruction fragments. Before: (XEN) d2v0 exception 6 (ec=0000) in emulation stub (line 6239) (XEN) d2v0 stub: c4 e1 44 77 c3 80 d0 82 ff ff ff d1 90 ec 90 After: (XEN) d3v0 exception 6 (ec=0000) in emulation stub (line 6239) (XEN) d3v0 stub: c4 e1 44 77 c3 cc cc cc cc cc cc cc cc cc cc Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> Semi-RFC: I really don't like (ab)use of memset, but can't think of a cleaner way of doing this. --- xen/arch/x86/x86_emulate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)