kernel July 2011

kernel@lists.fedoraproject.org

19 participants
30 discussions

Re: [PATCH 1/4] ptrace: temporary revert the recent ptrace/jobctl rework
by Oleg Nesterov 30 Jun '11

30 Jun '11

On 06/21, Roland McGrath wrote: > > > OK, so here's my (hacky) idea: > > (1) Forget ptrace-via-utrace. Have utrace be a separate thing. This > > way the recent ptrace changes won't matter. This is what V2 does. > > (2) But, what about ptrace co-existing well with utrace? Make them > > mutually exclusive - a ptraced-process can't be utraced and a > > utraced-process can't be ptraced. > > We had this situation before for a while. It has the substantial downside > that e.g. you cannot do any system-wide systemtap tracing without making > all strace and gdb use impossible. Yes, we can't make them mutually exclusive, this can't work. So V2 tries to teach them play well together. > > Assuming the above is a semi-reasonable idea, it might be a lot less > > work than updating the ptrace-via-utrace code to handle the new ptrace > > changes. > > That's for Oleg to say. (Sorry, Oleg. ;-) Oh, I am not sure what is simpler ;) Oleg.

1 0

[PATCH 19/19] utrace: consult PT_SINGLE_STEP/PT_SINGLE_BLOCK before user_disable_single_step()
by Oleg Nesterov 30 Jun '11

30 Jun '11

- do not do user_enable_block_step() if PT_SINGLE_STEP is set, in this case ptrace has already called user_enable_single_step() - do not do user_disable_single_step() if PT_*_STEP is set, ptrace needs the stepping Unless the tracee is killed this can't race with ptrace, this is called by the tracee itself. Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- kernel/utrace.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/utrace.c b/kernel/utrace.c index 508c13c..d6607cb 100644 --- a/kernel/utrace.c +++ b/kernel/utrace.c @@ -1828,7 +1828,8 @@ static void finish_resume_report(struct task_struct *task, case UTRACE_BLOCKSTEP: if (likely(arch_has_block_step())) { - user_enable_block_step(task); + if (!(current->ptrace & PT_SINGLE_STEP)) + user_enable_block_step(task); break; } @@ -1856,7 +1857,8 @@ static void finish_resume_report(struct task_struct *task, case UTRACE_REPORT: case UTRACE_RESUME: default: - user_disable_single_step(task); + if (!(current->ptrace & (PT_SINGLE_STEP | PT_SINGLE_BLOCK))) + user_disable_single_step(task); break; } } -- 1.5.5.1

1 0

[PATCH 18/19] introduce PT_SINGLE_STEP and PT_SINGLE_BLOCK
by Oleg Nesterov 30 Jun '11

30 Jun '11

Add the new internal ptrace flags, PT_SINGLE_STEP and PT_SINGLE_BLOCK. Like PT_SYSCALL_TRACE, this is needed to avoid the unnecessary ptrace reports when TIF_SINGLESTEP was set by another engine, not by ptrace. Also, we need these bits to coordinate the user_*_single_step() calls from ptrace and utrace. TODO: update the !x86 ptrace code which does user_disable_single_step(). Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- arch/x86/kernel/ptrace.c | 1 + include/linux/ptrace.h | 5 ++++- include/linux/tracehook.h | 7 +++++-- kernel/ptrace.c | 3 +++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c index 807c2a2..7ab475f 100644 --- a/arch/x86/kernel/ptrace.c +++ b/arch/x86/kernel/ptrace.c @@ -807,6 +807,7 @@ static int ioperm_get(struct task_struct *target, */ void ptrace_disable(struct task_struct *child) { + child->ptrace &= ~(PT_SINGLE_STEP | PT_SINGLE_BLOCK); user_disable_single_step(child); #ifdef TIF_SYSCALL_EMU clear_tsk_thread_flag(child, TIF_SYSCALL_EMU); diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index 98d995d..65b1e4f 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -91,6 +91,8 @@ #define PT_TRACE_MASK 0x000003f4 #define PT_SYSCALL_TRACE 0x00010000 +#define PT_SINGLE_STEP 0x00020000 +#define PT_SINGLE_BLOCK 0x00040000 /* single stepping state bits (used on ARM and PA-RISC) */ #define PT_SINGLESTEP_BIT 31 @@ -189,7 +191,8 @@ static inline void ptrace_init_task(struct task_struct *child, bool ptrace) child->ptrace = 0; if (unlikely(ptrace) && (current->ptrace & PT_PTRACED)) { child->ptrace = current->ptrace; - child->ptrace &= ~PT_SYSCALL_TRACE; + child->ptrace &= + ~(PT_SYSCALL_TRACE | PT_SINGLE_STEP | PT_SINGLE_BLOCK); __ptrace_link(child, current->parent); } diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h index 6ce7a37..06edb52 100644 --- a/include/linux/tracehook.h +++ b/include/linux/tracehook.h @@ -121,6 +121,9 @@ static inline __must_check int tracehook_report_syscall_entry( return 0; } +#define ptrace_wants_step() \ + (current->ptrace & (PT_SINGLE_STEP | PT_SINGLE_BLOCK)) + /** * tracehook_report_syscall_exit - task has just finished a system call * @regs: user register state of current task @@ -143,7 +146,7 @@ static inline void tracehook_report_syscall_exit(struct pt_regs *regs, int step) if (task_utrace_flags(current) & UTRACE_EVENT(SYSCALL_EXIT)) utrace_report_syscall_exit(regs); - if (step) { + if (step && ptrace_wants_step()) { siginfo_t info; user_single_step_siginfo(current, regs, &info); force_sig_info(SIGTRAP, &info, current); @@ -436,7 +439,7 @@ static inline void tracehook_signal_handler(int sig, siginfo_t *info, { if (task_utrace_flags(current)) utrace_signal_handler(current, stepping); - if (stepping) + if (stepping && ptrace_wants_step()) ptrace_notify(SIGTRAP); } diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 209ea2d..44908d0 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -575,13 +575,16 @@ static int ptrace_resume(struct task_struct *child, long request, clear_tsk_thread_flag(child, TIF_SYSCALL_EMU); #endif + child->ptrace &= ~(PT_SINGLE_STEP | PT_SINGLE_BLOCK); if (is_singleblock(request)) { if (unlikely(!arch_has_block_step())) return -EIO; + child->ptrace |= PT_SINGLE_BLOCK; user_enable_block_step(child); } else if (is_singlestep(request) || is_sysemu_singlestep(request)) { if (unlikely(!arch_has_single_step())) return -EIO; + child->ptrace |= PT_SINGLE_STEP; user_enable_single_step(child); } else { user_disable_single_step(child); -- 1.5.5.1

1 0

[PATCH 17/19] teach ptrace_set_syscall_trace() to play well with utrace
by Oleg Nesterov 30 Jun '11

30 Jun '11

1. ptrace_set_syscall_trace(true)->set_tsk_thread_flag(TIF_SYSCALL_TRACE) can race with utrace_control()->utrace_reset() path which can miss PT_SYSCALL_TRACE and clear TIF_SYSCALL_TRACE after it was already set. 2. ptrace_set_syscall_trace(false) clears TIF_SYSCALL_TRACE and this is not utrace-friendly, it can need this flag. Change ptrace_set_syscall_trace() to take task_utrace_lock(), this is enough to fix the 1st problem. Check task_utrace_flags() before clearing TIF_SYSCALL_TRACE, this fixes 2. Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- kernel/ptrace.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 0825a01..209ea2d 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -23,6 +23,7 @@ #include <linux/uaccess.h> #include <linux/regset.h> #include <linux/hw_breakpoint.h> +#include <linux/utrace.h> static void ptrace_signal_wake_up(struct task_struct *p, int quiescent) { @@ -39,13 +40,16 @@ static void ptrace_signal_wake_up(struct task_struct *p, int quiescent) static void ptrace_set_syscall_trace(struct task_struct *p, bool on) { + task_utrace_lock(p); if (on) { p->ptrace |= PT_SYSCALL_TRACE; set_tsk_thread_flag(p, TIF_SYSCALL_TRACE); } else { p->ptrace &= ~PT_SYSCALL_TRACE; - clear_tsk_thread_flag(p, TIF_SYSCALL_TRACE); + if (!(task_utrace_flags(p) & UTRACE_EVENT_SYSCALL)) + clear_tsk_thread_flag(p, TIF_SYSCALL_TRACE); } + task_utrace_unlock(p); } /* -- 1.5.5.1

1 0

[PATCH 16/19] introduce task_utrace_lock/task_utrace_unlock
by Oleg Nesterov 30 Jun '11

30 Jun '11

- Add task_utrace_lock(task). It simply takes task->utrace->lock if this task was ever utraced. Otherwise it takes task_lock(), this serializes with utrace_attach_task()->utrace_task_alloc(). In both case the caller can be sure it can't race with anything which needs utrace->lock. - Add task_utrace_unlock(task), it releases the corresponding lock. Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- include/linux/utrace.h | 9 +++++++++ kernel/utrace.c | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 0 deletions(-) diff --git a/include/linux/utrace.h b/include/linux/utrace.h index f251efe..5176f5f 100644 --- a/include/linux/utrace.h +++ b/include/linux/utrace.h @@ -109,6 +109,12 @@ void utrace_signal_handler(struct task_struct *, int); #ifndef CONFIG_UTRACE +static inline void task_utrace_lock(struct task_struct *task) +{ +} +static inline void task_utrace_unlock(struct task_struct *task) +{ +} /* * <linux/tracehook.h> uses these accessors to avoid #ifdef CONFIG_UTRACE. */ @@ -131,6 +137,9 @@ static inline void task_utrace_proc_status(struct seq_file *m, #else /* CONFIG_UTRACE */ +extern void task_utrace_lock(struct task_struct *task); +extern void task_utrace_unlock(struct task_struct *task); + static inline unsigned long task_utrace_flags(struct task_struct *task) { return task->utrace_flags; diff --git a/kernel/utrace.c b/kernel/utrace.c index a824ac3..508c13c 100644 --- a/kernel/utrace.c +++ b/kernel/utrace.c @@ -79,6 +79,32 @@ static int __init utrace_init(void) } module_init(utrace_init); +void task_utrace_lock(struct task_struct *task) +{ + struct utrace *utrace = task_utrace_struct(task); + + if (!utrace) { + task_lock(task); + utrace = task_utrace_struct(task); + if (!utrace) + return; + + task_unlock(task); + } + + spin_lock(&utrace->lock); +} + +void task_utrace_unlock(struct task_struct *task) +{ + struct utrace *utrace = task_utrace_struct(task); + + if (utrace) + spin_unlock(&utrace->lock); + else + task_unlock(task); +} + /* * Set up @task.utrace for the first time. We can have races * between two utrace_attach_task() calls here. The task_lock() -- 1.5.5.1

1 0

[PATCH 15/19] utrace: don't clear TIF_SYSCALL_TRACE if it is needed by ptrace
by Oleg Nesterov 30 Jun '11

30 Jun '11

TIF_SYSCALL_TRACE should be cleared only if both ptrace and utrace do not want it, change utrace_reset() to check PT_SYSCALL_TRACE before clear_tsk_thread_flag(TIF_SYSCALL_TRACE). Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- kernel/utrace.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/kernel/utrace.c b/kernel/utrace.c index daa96b9..a824ac3 100644 --- a/kernel/utrace.c +++ b/kernel/utrace.c @@ -697,6 +697,7 @@ static bool utrace_reset(struct task_struct *task, struct utrace *utrace) BUG_ON(utrace->death); flags &= UTRACE_EVENT(REAP); } else if (!(flags & UTRACE_EVENT_SYSCALL) && + !(task->ptrace & PT_SYSCALL_TRACE) && test_tsk_thread_flag(task, TIF_SYSCALL_TRACE)) { clear_tsk_thread_flag(task, TIF_SYSCALL_TRACE); } -- 1.5.5.1

1 0

[PATCH 14/19] introduce PT_SYSCALL_TRACE flag
by Oleg Nesterov 30 Jun '11

30 Jun '11

Currently tracehooks assume that if the ptraced task has TIF_SYSCALL_TRACE set, the tracee should report the syscall. This is not true, this thread flag can be set by utrace. Add the new internal ptrace flag, PT_SYSCALL_TRACE. Change ptrace_set_syscall_trace() to set/clear this bit along with TIF_SYSCALL_TRACE, change ptrace_report_syscall() to check this flag instead of PT_PTRACED. Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- include/linux/ptrace.h | 3 +++ include/linux/tracehook.h | 2 +- kernel/ptrace.c | 7 +++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index 9178d5c..98d995d 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -90,6 +90,8 @@ #define PT_TRACE_MASK 0x000003f4 +#define PT_SYSCALL_TRACE 0x00010000 + /* single stepping state bits (used on ARM and PA-RISC) */ #define PT_SINGLESTEP_BIT 31 #define PT_SINGLESTEP (1<<PT_SINGLESTEP_BIT) @@ -187,6 +189,7 @@ static inline void ptrace_init_task(struct task_struct *child, bool ptrace) child->ptrace = 0; if (unlikely(ptrace) && (current->ptrace & PT_PTRACED)) { child->ptrace = current->ptrace; + child->ptrace &= ~PT_SYSCALL_TRACE; __ptrace_link(child, current->parent); } diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h index 3c7b6b3..6ce7a37 100644 --- a/include/linux/tracehook.h +++ b/include/linux/tracehook.h @@ -76,7 +76,7 @@ static inline void ptrace_report_syscall(struct pt_regs *regs) { int ptrace = task_ptrace(current); - if (!(ptrace & PT_PTRACED)) + if (!(ptrace & PT_SYSCALL_TRACE)) return; ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0)); diff --git a/kernel/ptrace.c b/kernel/ptrace.c index b6fd922..0825a01 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -39,10 +39,13 @@ static void ptrace_signal_wake_up(struct task_struct *p, int quiescent) static void ptrace_set_syscall_trace(struct task_struct *p, bool on) { - if (on) + if (on) { + p->ptrace |= PT_SYSCALL_TRACE; set_tsk_thread_flag(p, TIF_SYSCALL_TRACE); - else + } else { + p->ptrace &= ~PT_SYSCALL_TRACE; clear_tsk_thread_flag(p, TIF_SYSCALL_TRACE); + } } /* -- 1.5.5.1

1 0

[PATCH 13/19] introduce ptrace_set_syscall_trace()
by Oleg Nesterov 30 Jun '11

30 Jun '11

No functional changes. Add the new helper, ptrace_set_syscall_trace(), which should be used to set/clear TIF_SYSCALL_TRACE in ptrace code. Currently it does nothing more. Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- kernel/ptrace.c | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-) diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 0b2aba5..b6fd922 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -37,6 +37,14 @@ static void ptrace_signal_wake_up(struct task_struct *p, int quiescent) kick_process(p); } +static void ptrace_set_syscall_trace(struct task_struct *p, bool on) +{ + if (on) + set_tsk_thread_flag(p, TIF_SYSCALL_TRACE); + else + clear_tsk_thread_flag(p, TIF_SYSCALL_TRACE); +} + /* * ptrace a task: make the debugger its new parent and * move it to the ptrace list. @@ -364,7 +372,7 @@ static int ptrace_detach(struct task_struct *child, unsigned int data) /* Architecture-specific hardware disable .. */ ptrace_disable(child); - clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE); + ptrace_set_syscall_trace(child, false); write_lock_irq(&tasklist_lock); /* @@ -551,10 +559,7 @@ static int ptrace_resume(struct task_struct *child, long request, if (!valid_signal(data)) return -EIO; - if (request == PTRACE_SYSCALL) - set_tsk_thread_flag(child, TIF_SYSCALL_TRACE); - else - clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE); + ptrace_set_syscall_trace(child, request == PTRACE_SYSCALL); #ifdef TIF_SYSCALL_EMU if (request == PTRACE_SYSEMU || request == PTRACE_SYSEMU_SINGLESTEP) -- 1.5.5.1

1 0

[PATCH 12/19] get_signal_to_deliver: restructure utrace/ptrace signal reporting
by Oleg Nesterov 30 Jun '11

30 Jun '11

get_signal_to_deliver() assumes that either tracehook_get_signal() does nothing (without CONFIG_UTRACE), or it also reports the signal to ptrace engine implemented on top of utrace. Now that ptrace works independently this doesn't work. Change the code to call ptrace_signal() after tracehook_get_signal(). Move ->ptrace check from ptrace_signal() to get_signal_to_deliver(), we do not want to change *return_ka if it was initialized by utrace and the task is not traced. IOW, roughly, ptrace acts as if it is the last attached engine, it takes the final decision about the signal. Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- kernel/signal.c | 24 +++++++++++------------- 1 files changed, 11 insertions(+), 13 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index 89e691d..d0e0c67 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2006,9 +2006,6 @@ retry: static int ptrace_signal(int signr, siginfo_t *info, struct pt_regs *regs, void *cookie) { - if (!task_ptrace(current)) - return signr; - ptrace_signal_deliver(regs, cookie); /* Let the debugger run. */ @@ -2110,6 +2107,7 @@ relock: signr = tracehook_get_signal(current, regs, info, return_ka); if (unlikely(signr < 0)) goto relock; + if (unlikely(signr != 0)) ka = return_ka; else { @@ -2117,18 +2115,18 @@ relock: GROUP_STOP_PENDING) && do_signal_stop(0)) goto relock; - signr = dequeue_signal(current, &current->blocked, - info); + signr = dequeue_signal(current, &current->blocked, info); - if (!signr) - break; /* will return 0 */ + ka = &sighand->action[signr-1]; + } - if (signr != SIGKILL) { - signr = ptrace_signal(signr, info, - regs, cookie); - if (!signr) - continue; - } + if (!signr) + break; /* will return 0 */ + + if (signr != SIGKILL && current->ptrace) { + signr = ptrace_signal(signr, info, regs, cookie); + if (!signr) + continue; ka = &sighand->action[signr-1]; } -- 1.5.5.1

1 0

[PATCH 11/19] ptrace_stop: do not assume the task is running after wake_up_quiescent()
by Oleg Nesterov 30 Jun '11

30 Jun '11

If ptrace_stop() sets TASK_TRACED and then detects we should not stop, it can race with utrace_do_stop() which can see TASK_TRACED and add TASK_UTRACED. In this case we should stop for utrace needs. Signed-off-by: Oleg Nesterov <oleg(a)redhat.com> --- kernel/signal.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index 57552e6..89e691d 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1829,6 +1829,14 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info) if (clear_code) current->exit_code = 0; read_unlock(&tasklist_lock); + + /* + * It is possible that __TASK_UTRACED was added by utrace + * while we were __TASK_TRACED and before we take ->siglock + * for wake_up_quiescent(), we need to block in this case. + * Otherwise this is unnecessary but absolutely harmless. + */ + schedule(); } tracehook_finish_stop(); -- 1.5.5.1

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

kernel July 2011