lkml.org 
[lkml]   [2011]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] Make PTRACE_SEIZE set ptrace options specified in 'data' parameter
On 09/08, Tejun Heo wrote:
>
> On Wed, Sep 07, 2011 at 11:40:31PM +0200, Denys Vlasenko wrote:
> > + if (seize) {
> > + if (addr != 0)
> > + goto out;
> > + if ((flags & ~(long)PTRACE_O_MASK) != PTRACE_SEIZE_DEVEL)
>
> Please use (unsigned long). Also, wouldn't it be better to do the
> following instead?
>
> if (!(flags & PTRACE_SEIZE_DEVEL))
> goto out;
> flags &= ~PTRACE_SEIZE_DEVEL;
>
> if ((flags & ~(unsigned long(PTRACE_O_MASK))))
> goto out;
>
> Then, we can later just delete the first three lines when removing
> SEIZE_DEVEL.
>
> > @@ -263,11 +272,9 @@ static int ptrace_attach(struct task_struct *task, long request,
> > if (task->ptrace)
> > goto unlock_tasklist;
> >
> > - task->ptrace = PT_PTRACED;
> > - if (seize)
> > - task->ptrace |= PT_SEIZED;
> > if (task_ns_capable(task, CAP_SYS_PTRACE))
> > - task->ptrace |= PT_PTRACE_CAP;
> > + flags |= PT_PTRACE_CAP;
> > + task->ptrace = flags;
>
> Can you please put this in a separate patch?

Yes.

> Hmm... also I think we
> probably want to set ->ptrace while holding siglock too.

I thought about this too, and I agree this makes sense

> There are
> places which assume ->ptrace is protected by siglock.

Really? Once again, I agree. But _afaics_ currently this is not strictly
needed. PT_PTRACED/PT_SEIZED should not go away under ->siglock, yes, but
it seems that it is fine to set them.

> and linking are
> protected by siglock

Hmm. Could you explain this? Why do want __ptrace_link under ->siglock ?

Oleg.



\
 
 \ /
  Last update: 2011-09-09 01:33    [W:1.211 / U:0.760 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site