lkml.org 
[lkml]   [2020]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 199/215] arm64: ptrace: Override SPSR.SS when single-stepping is enabled
    Date
    From: Will Deacon <will@kernel.org>

    commit 3a5a4366cecc25daa300b9a9174f7fdd352b9068 upstream.

    Luis reports that, when reverse debugging with GDB, single-step does not
    function as expected on arm64:

    | I've noticed, under very specific conditions, that a PTRACE_SINGLESTEP
    | request by GDB won't execute the underlying instruction. As a consequence,
    | the PC doesn't move, but we return a SIGTRAP just like we would for a
    | regular successful PTRACE_SINGLESTEP request.

    The underlying problem is that when the CPU register state is restored
    as part of a reverse step, the SPSR.SS bit is cleared and so the hardware
    single-step state can transition to the "active-pending" state, causing
    an unexpected step exception to be taken immediately if a step operation
    is attempted.

    In hindsight, we probably shouldn't have exposed SPSR.SS in the pstate
    accessible by the GPR regset, but it's a bit late for that now. Instead,
    simply prevent userspace from configuring the bit to a value which is
    inconsistent with the TIF_SINGLESTEP state for the task being traced.

    Cc: <stable@vger.kernel.org>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Keno Fischer <keno@juliacomputing.com>
    Link: https://lore.kernel.org/r/1eed6d69-d53d-9657-1fc9-c089be07f98c@linaro.org
    Reported-by: Luis Machado <luis.machado@linaro.org>
    Tested-by: Luis Machado <luis.machado@linaro.org>
    Signed-off-by: Will Deacon <will@kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    arch/arm64/include/asm/debug-monitors.h | 2 ++
    arch/arm64/kernel/debug-monitors.c | 20 ++++++++++++++++----
    arch/arm64/kernel/ptrace.c | 4 ++--
    3 files changed, 20 insertions(+), 6 deletions(-)

    --- a/arch/arm64/include/asm/debug-monitors.h
    +++ b/arch/arm64/include/asm/debug-monitors.h
    @@ -109,6 +109,8 @@ void disable_debug_monitors(enum dbg_act

    void user_rewind_single_step(struct task_struct *task);
    void user_fastforward_single_step(struct task_struct *task);
    +void user_regs_reset_single_step(struct user_pt_regs *regs,
    + struct task_struct *task);

    void kernel_enable_single_step(struct pt_regs *regs);
    void kernel_disable_single_step(void);
    --- a/arch/arm64/kernel/debug-monitors.c
    +++ b/arch/arm64/kernel/debug-monitors.c
    @@ -141,17 +141,20 @@ postcore_initcall(debug_monitors_init);
    /*
    * Single step API and exception handling.
    */
    -static void set_regs_spsr_ss(struct pt_regs *regs)
    +static void set_user_regs_spsr_ss(struct user_pt_regs *regs)
    {
    regs->pstate |= DBG_SPSR_SS;
    }
    -NOKPROBE_SYMBOL(set_regs_spsr_ss);
    +NOKPROBE_SYMBOL(set_user_regs_spsr_ss);

    -static void clear_regs_spsr_ss(struct pt_regs *regs)
    +static void clear_user_regs_spsr_ss(struct user_pt_regs *regs)
    {
    regs->pstate &= ~DBG_SPSR_SS;
    }
    -NOKPROBE_SYMBOL(clear_regs_spsr_ss);
    +NOKPROBE_SYMBOL(clear_user_regs_spsr_ss);
    +
    +#define set_regs_spsr_ss(r) set_user_regs_spsr_ss(&(r)->user_regs)
    +#define clear_regs_spsr_ss(r) clear_user_regs_spsr_ss(&(r)->user_regs)

    static DEFINE_SPINLOCK(debug_hook_lock);
    static LIST_HEAD(user_step_hook);
    @@ -404,6 +407,15 @@ void user_fastforward_single_step(struct
    clear_regs_spsr_ss(task_pt_regs(task));
    }

    +void user_regs_reset_single_step(struct user_pt_regs *regs,
    + struct task_struct *task)
    +{
    + if (test_tsk_thread_flag(task, TIF_SINGLESTEP))
    + set_user_regs_spsr_ss(regs);
    + else
    + clear_user_regs_spsr_ss(regs);
    +}
    +
    /* Kernel API */
    void kernel_enable_single_step(struct pt_regs *regs)
    {
    --- a/arch/arm64/kernel/ptrace.c
    +++ b/arch/arm64/kernel/ptrace.c
    @@ -1935,8 +1935,8 @@ static int valid_native_regs(struct user
    */
    int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task)
    {
    - if (!test_tsk_thread_flag(task, TIF_SINGLESTEP))
    - regs->pstate &= ~DBG_SPSR_SS;
    + /* https://lore.kernel.org/lkml/20191118131525.GA4180@willie-the-truck */
    + user_regs_reset_single_step(regs, task);

    if (is_compat_thread(task_thread_info(task)))
    return valid_compat_regs(regs);

    \
     
     \ /
      Last update: 2020-07-20 18:24    [W:2.298 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site