From e1f287735c1e58c653b516931b5d3dd899edcb77 Mon Sep 17 00:00:00 2001 From: Roland McGrath Date: Wed, 30 Jan 2008 13:30:50 +0100 Subject: x86 single_step: TIF_FORCED_TF This changes the single-step support to use a new thread_info flag TIF_FORCED_TF instead of the PT_DTRACE flag in task_struct.ptrace. This keeps arch implementation uses out of this non-arch field. This changes the ptrace access to eflags to mask TF and maintain the TIF_FORCED_TF flag directly if userland sets TF, instead of relying on ptrace_signal_deliver. The 64-bit and 32-bit kernels are harmonized on this same behavior. The ptrace_signal_deliver approach works now, but this change makes the low-level register access code reliable when called from different contexts than a ptrace stop, which will be possible in the future. The 64-bit do_debug exception handler is also changed not to clear TF from user-mode registers. This matches the 32-bit kernel's behavior. Signed-off-by: Roland McGrath Signed-off-by: Ingo Molnar Signed-off-by: Thomas Gleixner --- arch/x86/ia32/ptrace32.c | 20 ++++++++++++++++++-- arch/x86/kernel/process_32.c | 3 --- arch/x86/kernel/process_64.c | 5 ----- arch/x86/kernel/ptrace_32.c | 17 +++++++++++++++++ arch/x86/kernel/ptrace_64.c | 20 ++++++++++++++++++++ arch/x86/kernel/signal_32.c | 12 +++++------- arch/x86/kernel/signal_64.c | 14 +++++--------- arch/x86/kernel/step.c | 9 +++------ arch/x86/kernel/traps_64.c | 23 +++++------------------ include/asm-x86/signal.h | 11 ++--------- include/asm-x86/thread_info_32.h | 2 ++ include/asm-x86/thread_info_64.h | 2 ++ 12 files changed, 79 insertions(+), 59 deletions(-) diff --git a/arch/x86/ia32/ptrace32.c b/arch/x86/ia32/ptrace32.c index 9d754b640205..5dee33417313 100644 --- a/arch/x86/ia32/ptrace32.c +++ b/arch/x86/ia32/ptrace32.c @@ -89,6 +89,15 @@ static int putreg32(struct task_struct *child, unsigned regno, u32 val) __u64 *flags = &stack[offsetof(struct pt_regs, eflags)/8]; val &= FLAG_MASK; + /* + * If the user value contains TF, mark that + * it was not "us" (the debugger) that set it. + * If not, make sure it stays set if we had. + */ + if (val & X86_EFLAGS_TF) + clear_tsk_thread_flag(child, TIF_FORCED_TF); + else if (test_tsk_thread_flag(child, TIF_FORCED_TF)) + val |= X86_EFLAGS_TF; *flags = val | (*flags & ~FLAG_MASK); break; } @@ -179,9 +188,17 @@ static int getreg32(struct task_struct *child, unsigned regno, u32 *val) R32(eax, rax); R32(orig_eax, orig_rax); R32(eip, rip); - R32(eflags, eflags); R32(esp, rsp); + case offsetof(struct user32, regs.eflags): + /* + * If the debugger set TF, hide it from the readout. + */ + *val = stack[offsetof(struct pt_regs, eflags)/8]; + if (test_tsk_thread_flag(child, TIF_FORCED_TF)) + *val &= ~X86_EFLAGS_TF; + break; + case offsetof(struct user32, u_debugreg[0]): *val = child->thread.debugreg0; break; @@ -425,4 +442,3 @@ asmlinkage long sys32_ptrace(long request, u32 pid, u32 addr, u32 data) put_task_struct(child); return ret; } - diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index 4d66a56280d3..d9905c9d0fd5 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -817,9 +817,6 @@ asmlinkage int sys_execve(struct pt_regs regs) (char __user * __user *) regs.edx, ®s); if (error == 0) { - task_lock(current); - current->ptrace &= ~PT_DTRACE; - task_unlock(current); /* Make sure we don't return using sysenter.. */ set_thread_flag(TIF_IRET); } diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index ccc9d68d5a58..f7356e5517f6 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -709,11 +709,6 @@ long sys_execve(char __user *name, char __user * __user *argv, if (IS_ERR(filename)) return error; error = do_execve(filename, argv, envp, ®s); - if (error == 0) { - task_lock(current); - current->ptrace &= ~PT_DTRACE; - task_unlock(current); - } putname(filename); return error; } diff --git a/arch/x86/kernel/ptrace_32.c b/arch/x86/kernel/ptrace_32.c index b73960885c3f..bc7fd802dcc7 100644 --- a/arch/x86/kernel/ptrace_32.c +++ b/arch/x86/kernel/ptrace_32.c @@ -104,6 +104,15 @@ static int putreg(struct task_struct *child, break; case EFL: value &= FLAG_MASK; + /* + * If the user value contains TF, mark that + * it was not "us" (the debugger) that set it. + * If not, make sure it stays set if we had. + */ + if (value & X86_EFLAGS_TF) + clear_tsk_thread_flag(child, TIF_FORCED_TF); + else if (test_tsk_thread_flag(child, TIF_FORCED_TF)) + value |= X86_EFLAGS_TF; value |= get_stack_long(child, EFL_OFFSET) & ~FLAG_MASK; break; } @@ -119,6 +128,14 @@ static unsigned long getreg(struct task_struct *child, unsigned long retval = ~0UL; switch (regno >> 2) { + case EFL: + /* + * If the debugger set TF, hide it from the readout. + */ + retval = get_stack_long(child, EFL_OFFSET); + if (test_tsk_thread_flag(child, TIF_FORCED_TF)) + retval &= ~X86_EFLAGS_TF; + break; case GS: retval = child->thread.gs; break; diff --git a/arch/x86/kernel/ptrace_64.c b/arch/x86/kernel/ptrace_64.c index 4abfbced9b26..035d53e99c57 100644 --- a/arch/x86/kernel/ptrace_64.c +++ b/arch/x86/kernel/ptrace_64.c @@ -143,6 +143,15 @@ static int putreg(struct task_struct *child, return 0; case offsetof(struct user_regs_struct, eflags): value &= FLAG_MASK; + /* + * If the user value contains TF, mark that + * it was not "us" (the debugger) that set it. + * If not, make sure it stays set if we had. + */ + if (value & X86_EFLAGS_TF) + clear_tsk_thread_flag(child, TIF_FORCED_TF); + else if (test_tsk_thread_flag(child, TIF_FORCED_TF)) + value |= X86_EFLAGS_TF; tmp = get_stack_long(child, EFL_OFFSET); tmp &= ~FLAG_MASK; value |= tmp; @@ -189,6 +198,17 @@ static unsigned long getreg(struct task_struct *child, unsigned long regno) if (child->thread.gsindex != GS_TLS_SEL) return 0; return get_desc_base(&child->thread.tls_array[GS_TLS]); + case offsetof(struct user_regs_struct, eflags): + /* + * If the debugger set TF, hide it from the readout. + */ + regno = regno - sizeof(struct pt_regs); + val = get_stack_long(child, regno); + if (test_tsk_thread_flag(child, TIF_IA32)) + val &= 0xffffffff; + if (test_tsk_thread_flag(child, TIF_FORCED_TF)) + val &= ~X86_EFLAGS_TF; + return val; default: regno = regno - sizeof(struct pt_regs); val = get_stack_long(child, regno); diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c index 1ac53e9a0859..0a7c812212c9 100644 --- a/arch/x86/kernel/signal_32.c +++ b/arch/x86/kernel/signal_32.c @@ -545,14 +545,12 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka, } /* - * If TF is set due to a debugger (PT_DTRACE), clear the TF flag so - * that register information in the sigcontext is correct. + * If TF is set due to a debugger (TIF_FORCED_TF), clear the TF + * flag so that register information in the sigcontext is correct. */ - if (unlikely(regs->eflags & TF_MASK) - && likely(current->ptrace & PT_DTRACE)) { - current->ptrace &= ~PT_DTRACE; - regs->eflags &= ~TF_MASK; - } + if (unlikely(regs->eflags & X86_EFLAGS_TF) && + likely(test_and_clear_thread_flag(TIF_FORCED_TF))) + regs->eflags &= ~X86_EFLAGS_TF; /* Set up the stack frame */ if (ka->sa.sa_flags & SA_SIGINFO) diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c index 38d806467c0f..ab0178ebe00a 100644 --- a/arch/x86/kernel/signal_64.c +++ b/arch/x86/kernel/signal_64.c @@ -349,16 +349,12 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka, } /* - * If TF is set due to a debugger (PT_DTRACE), clear the TF - * flag so that register information in the sigcontext is - * correct. + * If TF is set due to a debugger (TIF_FORCED_TF), clear the TF + * flag so that register information in the sigcontext is correct. */ - if (unlikely(regs->eflags & TF_MASK)) { - if (likely(current->ptrace & PT_DTRACE)) { - current->ptrace &= ~PT_DTRACE; - regs->eflags &= ~TF_MASK; - } - } + if (unlikely(regs->eflags & X86_EFLAGS_TF) && + likely(test_and_clear_thread_flag(TIF_FORCED_TF))) + regs->eflags &= ~X86_EFLAGS_TF; #ifdef CONFIG_IA32_EMULATION if (test_thread_flag(TIF_IA32)) { diff --git a/arch/x86/kernel/step.c b/arch/x86/kernel/step.c index 6732272e3479..243bff650ca5 100644 --- a/arch/x86/kernel/step.c +++ b/arch/x86/kernel/step.c @@ -135,7 +135,7 @@ void user_enable_single_step(struct task_struct *child) if (is_setting_trap_flag(child, regs)) return; - child->ptrace |= PT_DTRACE; + set_tsk_thread_flag(child, TIF_FORCED_TF); } void user_disable_single_step(struct task_struct *child) @@ -144,9 +144,6 @@ void user_disable_single_step(struct task_struct *child) clear_tsk_thread_flag(child, TIF_SINGLESTEP); /* But touch TF only if it was set by us.. */ - if (child->ptrace & PT_DTRACE) { - struct pt_regs *regs = task_pt_regs(child); - regs->eflags &= ~X86_EFLAGS_TF; - child->ptrace &= ~PT_DTRACE; - } + if (test_and_clear_tsk_thread_flag(child, TIF_FORCED_TF)) + task_pt_regs(child)->eflags &= ~X86_EFLAGS_TF; } diff --git a/arch/x86/kernel/traps_64.c b/arch/x86/kernel/traps_64.c index aa248d754533..874aca397b02 100644 --- a/arch/x86/kernel/traps_64.c +++ b/arch/x86/kernel/traps_64.c @@ -865,27 +865,14 @@ asmlinkage void __kprobes do_debug(struct pt_regs * regs, tsk->thread.debugreg6 = condition; - /* Mask out spurious TF errors due to lazy TF clearing */ + + /* + * Single-stepping through TF: make sure we ignore any events in + * kernel space (but re-enable TF when returning to user mode). + */ if (condition & DR_STEP) { - /* - * The TF error should be masked out only if the current - * process is not traced and if the TRAP flag has been set - * previously by a tracing process (condition detected by - * the PT_DTRACE flag); remember that the i386 TRAP flag - * can be modified by the process itself in user mode, - * allowing programs to debug themselves without the ptrace() - * interface. - */ if (!user_mode(regs)) goto clear_TF_reenable; - /* - * Was the TF flag set by a debugger? If so, clear it now, - * so that register information is correct. - */ - if (tsk->ptrace & PT_DTRACE) { - regs->eflags &= ~TF_MASK; - tsk->ptrace &= ~PT_DTRACE; - } } /* Ok, finally something we can handle */ diff --git a/include/asm-x86/signal.h b/include/asm-x86/signal.h index 987a422a2c78..aee7eca585ab 100644 --- a/include/asm-x86/signal.h +++ b/include/asm-x86/signal.h @@ -245,21 +245,14 @@ static __inline__ int sigfindinword(unsigned long word) struct pt_regs; -#define ptrace_signal_deliver(regs, cookie) \ - do { \ - if (current->ptrace & PT_DTRACE) { \ - current->ptrace &= ~PT_DTRACE; \ - (regs)->eflags &= ~TF_MASK; \ - } \ - } while (0) - #else /* __i386__ */ #undef __HAVE_ARCH_SIG_BITOPS +#endif /* !__i386__ */ + #define ptrace_signal_deliver(regs, cookie) do { } while (0) -#endif /* !__i386__ */ #endif /* __KERNEL__ */ #endif /* __ASSEMBLY__ */ diff --git a/include/asm-x86/thread_info_32.h b/include/asm-x86/thread_info_32.h index a516e9192f11..009ecc6ad38b 100644 --- a/include/asm-x86/thread_info_32.h +++ b/include/asm-x86/thread_info_32.h @@ -138,6 +138,7 @@ static inline struct thread_info *current_thread_info(void) #define TIF_IO_BITMAP 18 /* uses I/O bitmap */ #define TIF_FREEZE 19 /* is freezing for suspend */ #define TIF_NOTSC 20 /* TSC is not accessible in userland */ +#define TIF_FORCED_TF 21 /* true if TF in eflags artificially */ #define _TIF_SYSCALL_TRACE (1<