Re: [PATCH 1/3] ptrace: ptrace_attach: check PF_KTHREAD +exit_state instead of ->mm

[Roland McGrath]

> > On second look, what does __ptrace_may_access() need task_lock() for anyway?
> 
> Just for get_dumpable(task->mm), I think.

Ah.  That's really only for using the mm, right?  i.e. it could be using
get_task_mm() instead.  set_dumpable() does not use task_lock() to
synchronize the actual changes that affect get_dumpable().

> OK, so this change is not purely cosmetic as I thought.

Right.

> We can fix this in many ways. We can extract the ->cred and ->mm checks
> from __ptrace_may_access() into another helper which is called before
> write_lock(tasklist), and then call security_ptrace_may_access under tasklist.
> Or we can do get_task_mm() in advance and call __ptrace_may_access() without
> task_lock().

get_task_mm() makes sense to me.  It seems like those checks being under
the tasklist_lock (i.e. the lock governing ptrace attach) might matter.

> Or, perhaps, we can just check ->ptrace before __ptrace_may_access()
> lockless (just to prevent the scenario above), and then check it again
> under tasklist? This looks like a simplest option.

Doesn't that still have a possible race with PTRACE_TRACEME?  If
ptrace_traceme() runs between the lockless check and the lock-taking, we
want PTRACE_ATTACH to fail without calling security_ptrace_may_access().
Maybe the cred mutex should be excluding that here anyway.


Thanks,
Roland
--
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/