mbox series

[0/5] Fixes to debugging facilities

Message ID 5fa229fc-9514-abc6-5e72-2447a2c637d0@gmail.com (mailing list archive)
Headers show
Series Fixes to debugging facilities | expand

Message

Jinoh Kang Aug. 21, 2023, 3:55 p.m. UTC 
This is a rebased version of Andrew Cooper's debugging facilities patch:
https://lore.kernel.org/xen-devel/1528120755-17455-1-git-send-email-andrew.cooper3@citrix.com/

> So this started as a small fix for the vmentry failure (penultimate patch),
> and has snowballed...
>
> I'm fairly confident that everything involving DEBUGCTL.BTF is broken, and
> there are definitely bugs with configuring DEBUGCTL.RTM (which really isn't
> helped by the fact that the GCC TSX intrinsics render the resulting code
> un-debuggable.)  I'll defer fixing these swamps for now.
>
> The first 4 patches probably want backporting to the stable trees, so I've
> taken care to move them ahead of patch 6 for backport reasons.  While all
> fixes would ideally be backported, I can't find a way of fixing %dr6 merging
> (as it needs to be done precicely once) without a behavioural change in the
> monitor subsystem.
>
> Patch 8 probably breaks introspection, so can't be taken at this point.  See
> that patch for discussion of the problem and my best guess at a solution.

6 out of 11 patches from the 2018 patch series above, including the
vmentry failure fix, have already been committed.  This covers the
remaining 5 patches.

One particular bug that the patch series fixes involves simultaneous
hardware breakpoint exception and single-stepping exception occurring at
the same PC (IP).  Xen blindly sets singlestep (DR6.BS := 1) in this
case, which interferes with userland debugging and allows (otherwise
restricted) usermode programs to detect Xen HVM (or PVH).  The following
Linux x86-64 program demonstrates the bug:

-----------------------------------8<-----------------------------------

#include <stddef.h>
#include <stdlib.h>
#include <assert.h>
#include <unistd.h>
#include <signal.h>
#include <sys/ptrace.h>
#include <sys/wait.h>
#include <sys/user.h>
#include <stdio.h>

#define ABORT_ON_ERR(x) if ((x) == -1) abort();

int main(void)
{
    unsigned long cur_rip, cur_eflags, cur_dr6;
    int wstatus, exit_code;
    pid_t pid;

    ABORT_ON_ERR(pid = fork());
    if (pid == 0) {
        ABORT_ON_ERR(ptrace(PTRACE_TRACEME, 0, NULL, NULL));
        ABORT_ON_ERR(raise(SIGSTOP));
        _exit(0);
    }

    /* Wait for first ptrace event */
    if (waitpid(pid, &wstatus, 0) != pid) abort();
    if (!WIFSTOPPED(wstatus)) abort();

    /* Obtain current RIP value and perform sanity check */
    cur_rip = ptrace(PTRACE_PEEKUSER, pid, (void *)offsetof(struct user, regs.rip), &cur_rip);
    cur_dr6 = ptrace(PTRACE_PEEKUSER, pid, (void *)offsetof(struct user, u_debugreg[6]), &cur_dr6);
    assert(cur_dr6 == 0xffff0ff0UL);

    /* Set up debug registers and set EFLAGS.TF */
    cur_eflags = ptrace(PTRACE_PEEKUSER, pid, (void *)offsetof(struct user, regs.eflags), &cur_eflags);
    ABORT_ON_ERR(ptrace(PTRACE_POKEUSER, pid, (void *)offsetof(struct user, regs.eflags), (void *)(cur_eflags | 0x100UL)));
    ABORT_ON_ERR(ptrace(PTRACE_POKEUSER, pid, (void *)offsetof(struct user, u_debugreg[0]), (void *)cur_rip));
    ABORT_ON_ERR(ptrace(PTRACE_POKEUSER, pid, (void *)offsetof(struct user, u_debugreg[7]), (void *)1L));

    /* Continue execution to trigger hardware breakpoint */
    ABORT_ON_ERR(ptrace(PTRACE_CONT, pid, NULL, (unsigned long)0));
    if (waitpid(pid, &wstatus, 0) != pid) abort();
    if (!(WIFSTOPPED(wstatus) && WSTOPSIG(wstatus) == SIGTRAP)) abort();

    /* Detect if Xen has tampered with DR6 */
    cur_dr6 = ptrace(PTRACE_PEEKUSER, pid, (void *)offsetof(struct user, u_debugreg[6]), &cur_dr6);
    fprintf(stderr, "DR6 = 0x%08lx\n", cur_dr6);
    if (cur_dr6 == 0xffff0ff1UL)
    {
        fputs("Running on bare-metal, Xen PV, or non-Xen VMM\n", stdout);
        exit_code = EXIT_FAILURE;
    }
    else
    {
        fputs("Running on Xen HVM\n", stdout);
        exit_code = EXIT_SUCCESS;
    }

    /* Tear down debug registers and unset EFLAGS.TF */
    cur_eflags = ptrace(PTRACE_PEEKUSER, pid, (void *)offsetof(struct user, regs.eflags), &cur_eflags);
    ABORT_ON_ERR(ptrace(PTRACE_POKEUSER, pid, (void *)offsetof(struct user, regs.eflags), (void *)(cur_eflags & ~0x100UL)));
    ABORT_ON_ERR(ptrace(PTRACE_POKEUSER, pid, (void *)offsetof(struct user, u_debugreg[7]), (void *)0L));

    /* Continue execution to let child process exit */
    ABORT_ON_ERR(ptrace(PTRACE_CONT, pid, NULL, (unsigned long)0));
    if (waitpid(pid, &wstatus, 0) != pid) abort();
    if (!(WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == 0)) abort();

    return exit_code;
}

-----------------------------------8<-----------------------------------

Andrew Cooper (5):
  x86: Fix calculation of %dr6/7 reserved bits
  x86/emul: Add pending_dbg field to x86_event
  x86/hvm: RFC - PROBABLY BROKEN - Defer all debugging/monitor actions
    to {svm,vmx}_inject_event()
  x86: Fix merging of new status bits into %dr6
  x86/dbg: Cleanup of legacy dr6 constants

 xen/arch/x86/domain.c                  |   7 +-
 xen/arch/x86/hvm/emulate.c             |   3 +-
 xen/arch/x86/hvm/hvm.c                 |   8 +-
 xen/arch/x86/hvm/svm/svm.c             | 126 ++++++++++++++-----------
 xen/arch/x86/hvm/vmx/vmx.c             | 112 ++++++++++------------
 xen/arch/x86/include/asm/debugreg.h    |  94 +++++++++++++-----
 xen/arch/x86/include/asm/domain.h      |  12 +++
 xen/arch/x86/include/asm/hvm/hvm.h     |  15 ++-
 xen/arch/x86/include/asm/x86-defns.h   |  10 --
 xen/arch/x86/mm/shadow/multi.c         |   5 +-
 xen/arch/x86/pv/emul-priv-op.c         |  13 ++-
 xen/arch/x86/pv/emulate.c              |   6 +-
 xen/arch/x86/pv/misc-hypercalls.c      |  16 +---
 xen/arch/x86/pv/ro-page-fault.c        |   3 +-
 xen/arch/x86/pv/traps.c                |  17 +++-
 xen/arch/x86/traps.c                   |   8 +-
 xen/arch/x86/x86_emulate/x86_emulate.h |   5 +-
 17 files changed, 262 insertions(+), 198 deletions(-)

Comments

Jan Beulich Aug. 22, 2023, 6:16 a.m. UTC | #1 
On 21.08.2023 17:55, Jinoh Kang wrote:
> This is a rebased version of Andrew Cooper's debugging facilities patch:
> https://lore.kernel.org/xen-devel/1528120755-17455-1-git-send-email-andrew.cooper3@citrix.com/
> 
>> So this started as a small fix for the vmentry failure (penultimate patch),
>> and has snowballed...
>>
>> I'm fairly confident that everything involving DEBUGCTL.BTF is broken, and
>> there are definitely bugs with configuring DEBUGCTL.RTM (which really isn't
>> helped by the fact that the GCC TSX intrinsics render the resulting code
>> un-debuggable.)  I'll defer fixing these swamps for now.
>>
>> The first 4 patches probably want backporting to the stable trees, so I've
>> taken care to move them ahead of patch 6 for backport reasons.  While all
>> fixes would ideally be backported, I can't find a way of fixing %dr6 merging
>> (as it needs to be done precicely once) without a behavioural change in the
>> monitor subsystem.
>>
>> Patch 8 probably breaks introspection, so can't be taken at this point.  See
>> that patch for discussion of the problem and my best guess at a solution.
> 
> 6 out of 11 patches from the 2018 patch series above, including the
> vmentry failure fix, have already been committed.  This covers the
> remaining 5 patches.

One important formal question: Where did Andrew's S-o-b go on all of the
patches?

Jan
Jinoh Kang Aug. 22, 2023, 2:55 p.m. UTC | #2 
On 8/22/23 15:16, Jan Beulich wrote:
> One important formal question: Where did Andrew's S-o-b go on all of the
> patches?

Thanks for catching it.  Looks like I confused the submission process with that
of another (non-LF) project.  I'll re-read the docs to see if I missed something
else w.r.t. tagging rules.

> 
> Jan