On 07/24, Roland McGrath wrote:
>
> > For example, wait_consider_task(). Even some tracehooks, say,
> > tracehook_notify_death() need task_is_ptraced().
>
> Not "some", just that one, right?
Not just one. tracehook_tracer_task, tracehook_notify_jctl.
But,
> So these are all really the same one
> thing: ptrace interfering with normal parent wait/SIGCHLD.
yes, they are the same thing actually.
> So I'd do:
>
> static inline bool tracehook_inhibit_wait(struct task_struct *task)
> {
> return task->utrace_flags && utrace_inhibit_wait(task);
> }
>
> and then utrace_inhibit_wait can look at a bit we keep in struct utrace or
> something like that. I think we might be able to get away with unlocked
> checks of that bit in wait, if we're sure that we'll do an extra
> wake_up_parent after detach or whatnot.
Yes, I thought about something like this too.
The problem is, I can't imagine all details now. And. I think it is better
to make the first working version as simple/small as possible.
So. For the first version of CONFIG_PTRACE_UTRACE I'd suggest to use
->ptrace to keep the single bit, PT_PTRACED (well, PT_PTRACE_CAP too).
All other PT_ bits should go into engine->data.
Also. Either we have to add a lot of #ifdef's into tracehook.h, or we
can add a single #ifdef into task_ptrace() (like your utrace-ptrace.patch
does). In the last case, do you think something like below makes sense?
is_task_ptraced() - means the same with or without utrace
task_ptrace() - returns 0 if CONFIG_UTRACE
Or we can start with reverting 5cb11446892833e50970fb2277a9f7563b0a8bd3
I regret about this patch now (and now I understand why you were not
very happy with this change ;). But this patch was needed for child-data
cleanups. However, I think we should forget about these cleanups for now.
In any case, we need some changes, and the patch should be sent separately,
before re-sending the updated version of utrace, afaics.
Oleg.
--- WAIT/include/linux/ptrace.h~1_IS_PTRACED 2009-06-19 01:12:47.000000000
+0200
+++ WAIT/include/linux/ptrace.h 2009-08-03 21:49:19.000000000 +0200
@@ -120,6 +120,11 @@ static inline void ptrace_unlink(struct
int generic_ptrace_peekdata(struct task_struct *tsk, long addr, long data);
int generic_ptrace_pokedata(struct task_struct *tsk, long addr, long data);
+static inline bool is_task_ptraced(struct task_struct *task)
+{
+ return task->ptrace != 0;
+}
+
/**
* task_ptrace - return %PT_* flags that apply to a task
* @task: pointer to &task_struct in question
--- WAIT/include/linux/tracehook.h~1_IS_PTRACED 2009-06-19 01:12:47.000000000
+0200
+++ WAIT/include/linux/tracehook.h 2009-08-03 22:57:38.000000000 +0200
@@ -63,7 +63,7 @@ struct linux_binprm;
*/
static inline int tracehook_expect_breakpoints(struct task_struct *task)
{
- return (task_ptrace(task) & PT_PTRACED) != 0;
+ return task_ptrace(task) != 0;
}
/*
@@ -73,7 +73,7 @@ static inline void ptrace_report_syscall
{
int ptrace = task_ptrace(current);
- if (!(ptrace & PT_PTRACED))
+ if (!ptrace)
return;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
@@ -148,9 +148,8 @@ static inline void tracehook_report_sysc
static inline int tracehook_unsafe_exec(struct task_struct *task)
{
int unsafe = 0;
- int ptrace = task_ptrace(task);
- if (ptrace & PT_PTRACED) {
- if (ptrace & PT_PTRACE_CAP)
+ if (is_task_ptraced(task)) {
+ if (task->ptrace & PT_PTRACE_CAP)
unsafe |= LSM_UNSAFE_PTRACE_CAP;
else
unsafe |= LSM_UNSAFE_PTRACE;
@@ -171,7 +170,7 @@ static inline int tracehook_unsafe_exec(
*/
static inline struct task_struct *tracehook_tracer_task(struct task_struct
*tsk)
{
- if (task_ptrace(tsk) & PT_PTRACED)
+ if (is_task_ptraced(tsk))
return rcu_dereference(tsk->parent);
return NULL;
}
@@ -195,7 +194,7 @@ static inline void tracehook_report_exec
struct pt_regs *regs)
{
if (!ptrace_event(PT_TRACE_EXEC, PTRACE_EVENT_EXEC, 0) &&
- unlikely(task_ptrace(current) & PT_PTRACED))
+ unlikely(task_ptrace(current))
send_sig(SIGTRAP, current, 0);
}
@@ -379,7 +378,7 @@ static inline void tracehook_signal_hand
const struct k_sigaction *ka,
struct pt_regs *regs, int stepping)
{
- if (stepping)
+ if (task_ptrace(current) && stepping)
ptrace_notify(SIGTRAP);
}
@@ -396,7 +395,7 @@ static inline void tracehook_signal_hand
static inline int tracehook_consider_ignored_signal(struct task_struct *task,
int sig)
{
- return (task_ptrace(task) & PT_PTRACED) != 0;
+ return task_ptrace(task) != 0;
}
/**
@@ -415,7 +414,7 @@ static inline int tracehook_consider_ign
static inline int tracehook_consider_fatal_signal(struct task_struct *task,
int sig)
{
- return (task_ptrace(task) & PT_PTRACED) != 0;
+ return task_ptrace(task) != 0;
}
/**
@@ -478,7 +477,7 @@ static inline int tracehook_get_signal(s
*/
static inline int tracehook_notify_jctl(int notify, int why)
{
- return notify || (current->ptrace & PT_PTRACED);
+ return notify || is_task_ptraced(current);
}
#define DEATH_REAP -1
@@ -502,7 +501,7 @@ static inline int tracehook_notify_death
void **death_cookie, int group_dead)
{
if (task_detached(task))
- return task->ptrace ? SIGCHLD : DEATH_REAP;
+ return is_task_ptraced(task) ? SIGCHLD : DEATH_REAP;
/*
* If something other than our normal parent is ptracing us, then
@@ -512,7 +511,7 @@ static inline int tracehook_notify_death
if (thread_group_empty(task) && !ptrace_reparented(task))
return task->exit_signal;
- return task->ptrace ? SIGCHLD : DEATH_DELAYED_GROUP_LEADER;
+ return is_task_ptraced(task) ? SIGCHLD : DEATH_DELAYED_GROUP_LEADER;
}
/**
--- WAIT/kernel/ptrace.c~1_IS_PTRACED 2009-07-01 20:52:58.000000000 +0200
+++ WAIT/kernel/ptrace.c 2009-08-03 22:56:36.000000000 +0200
@@ -95,7 +95,7 @@ int ptrace_check_attach(struct task_stru
* be changed by us so it's not changing right after this.
*/
read_lock(&tasklist_lock);
- if ((child->ptrace & PT_PTRACED) && child->parent == current) {
+ if (child->ptrace && child->parent == current) {
ret = 0;
/*
* child->sighand can't be NULL, release_task()
--- WAIT/kernel/exit.c~1_IS_PTRACED 2009-07-02 20:19:55.000000000 +0200
+++ WAIT/kernel/exit.c 2009-08-03 22:41:19.000000000 +0200
@@ -754,7 +754,7 @@ static void reparent_leader(struct task_
p->exit_signal = SIGCHLD;
/* If it has exited notify the new parent about this child's death. */
- if (!task_ptrace(p) &&
+ if (!is_task_ptraced(p) &&
p->exit_state == EXIT_ZOMBIE && thread_group_empty(p)) {
do_notify_parent(p, p->exit_signal);
if (task_detached(p)) {
@@ -781,7 +781,7 @@ static void forget_original_parent(struc
do {
t->real_parent = reaper;
if (t->parent == father) {
- BUG_ON(task_ptrace(t));
+ BUG_ON(is_task_ptraced(t));
t->parent = t->real_parent;
}
if (t->pdeath_signal)
@@ -1492,7 +1492,7 @@ static int wait_consider_task(struct wai
return 0;
}
- if (likely(!ptrace) && unlikely(task_ptrace(p))) {
+ if (likely(!ptrace) && unlikely(is_task_ptraced(p))) {
/*
* This child is hidden by ptrace.
* We aren't allowed to see it now, but eventually we will.
--- WAIT/kernel/signal.c~1_IS_PTRACED 2009-08-01 02:21:27.000000000 +0200
+++ WAIT/kernel/signal.c 2009-08-03 22:42:20.000000000 +0200
@@ -1398,7 +1398,7 @@ int do_notify_parent(struct task_struct
/* do_notify_parent_cldstop should have been called instead. */
BUG_ON(task_is_stopped_or_traced(tsk));
- BUG_ON(!task_ptrace(tsk) &&
+ BUG_ON(!is_task_ptraced(tsk) &&
(tsk->group_leader != tsk || !thread_group_empty(tsk)));
info.si_signo = sig;
@@ -1437,7 +1437,7 @@ int do_notify_parent(struct task_struct
psig = tsk->parent->sighand;
spin_lock_irqsave(&psig->siglock, flags);
- if (!task_ptrace(tsk) && sig == SIGCHLD &&
+ if (!is_task_ptraced(tsk) && sig == SIGCHLD &&
(psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
(psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
/*
@@ -1474,7 +1474,7 @@ static void do_notify_parent_cldstop(str
struct task_struct *parent;
struct sighand_struct *sighand;
- if (task_ptrace(tsk))
+ if (is_task_ptraced(tsk))
parent = tsk->parent;
else {
tsk = tsk->group_leader;
