Re: [PATCH] ptrace: do not use task->ptrace directly in core kernel

From: Oleg Nesterov
Date: Sun May 03 2009 - 15:58:42 EST


On 04/30, Roland McGrath wrote:
>
> That is fine, but doesn't buy much.

Yes this patch is only cosmetic. But it allows us to change the internals
in ptrace.c without thinking about other kernel/*.c. The only goal is to
make the further changes smaller and more reviewable.

Honestly, I'd also like to temporary change ptrace.c too, see the patch
below. But this is minor.

> i.e., we will be changing these again
> before too long anyway I imagine.

Yes, agreed.

> The BUG_ON cases might as well just go away, probably.

Agreed.

> ptrace_fork() is a wrapper that just calls arch_ptrace_fork(), which itself
> is an empty macro on most configurations. I think we might as well just
> make ptrace_fork() an inline in linux/ptrace.h and put the test inside it.
> (Thus any future changes touch ptrace.h and not fork.c.)

ptrace_fork() will die soon, so the change if fork.c will go away. Markus
already has patches, hopefully they will be merged soon.

Oleg.

--- a/kernel/ptrace.c 2009-05-03 19:57:11.000000000 +0200
+++ b/kernel/ptrace.c 2009-05-03 21:24:36.000000000 +0200
@@ -79,8 +79,6 @@ static void ptrace_untrace(struct task_s
*/
void __ptrace_unlink(struct task_struct *child)
{
- BUG_ON(!child->ptrace);
-
child->ptrace = 0;
child->parent = child->real_parent;
list_del_init(&child->ptrace_entry);
@@ -105,7 +103,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 (task_ptrace(child) && child->parent == current) {
ret = 0;
/*
* child->sighand can't be NULL, release_task()
@@ -203,7 +201,7 @@ int ptrace_attach(struct task_struct *ta
retval = -EPERM;
if (unlikely(task->exit_state))
goto unlock_tasklist;
- if (task->ptrace)
+ if (task_ptrace(task))
goto unlock_tasklist;

task->ptrace = PT_PTRACED;
@@ -233,7 +231,7 @@ int ptrace_traceme(void)

write_lock_irq(&tasklist_lock);
/* Are we already being traced? */
- if (!current->ptrace) {
+ if (!task_ptrace(current)) {
ret = security_ptrace_traceme(current->parent);
/*
* Check PF_EXITING to ensure ->real_parent has not passed
@@ -314,7 +312,7 @@ int ptrace_detach(struct task_struct *ch
* This child can be already killed. Make sure de_thread() or
* our sub-thread doing do_wait() didn't do release_task() yet.
*/
- if (child->ptrace) {
+ if (task_ptrace(child)) {
child->exit_code = data;
dead = __ptrace_detach(current, child);
}
@@ -401,30 +399,35 @@ int ptrace_writedata(struct task_struct

static int ptrace_setoptions(struct task_struct *child, long data)
{
- child->ptrace &= ~PT_TRACE_MASK;
+ unsigned int new_flags = 0;
+
+ if (data & ~PTRACE_O_MASK)
+ return -EINVAL;

if (data & PTRACE_O_TRACESYSGOOD)
- child->ptrace |= PT_TRACESYSGOOD;
+ new_flags |= PT_TRACESYSGOOD;

if (data & PTRACE_O_TRACEFORK)
- child->ptrace |= PT_TRACE_FORK;
+ new_flags |= PT_TRACE_FORK;

if (data & PTRACE_O_TRACEVFORK)
- child->ptrace |= PT_TRACE_VFORK;
+ new_flags |= PT_TRACE_VFORK;

if (data & PTRACE_O_TRACECLONE)
- child->ptrace |= PT_TRACE_CLONE;
+ new_flags |= PT_TRACE_CLONE;

if (data & PTRACE_O_TRACEEXEC)
- child->ptrace |= PT_TRACE_EXEC;
+ new_flags |= PT_TRACE_EXEC;

if (data & PTRACE_O_TRACEVFORKDONE)
- child->ptrace |= PT_TRACE_VFORK_DONE;
+ new_flags |= PT_TRACE_VFORK_DONE;

if (data & PTRACE_O_TRACEEXIT)
- child->ptrace |= PT_TRACE_EXIT;
+ new_flags |= PT_TRACE_EXIT;

- return (data & ~PTRACE_O_MASK) ? -EINVAL : 0;
+ child->ptrace &= ~PT_TRACE_MASK;
+ child->ptrace |= new_flags;
+ return 0;
}

static int ptrace_getsiginfo(struct task_struct *child, siginfo_t *info)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/