lkml.org 
[lkml]   [2009]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/3] ptrace: do not use task_lock() for attach
    * Oleg Nesterov (oleg@redhat.com) wrote:
    > + write_lock_irq(&tasklist_lock);
    > retval = -EPERM;
    > if (unlikely(task->exit_state))
    > - goto bad;
    > + goto unlock_tasklist;
    > if (task->ptrace)
    > - goto bad;
    > + goto unlock_tasklist;

    So, task->ptrace now protected by tasklist_lock to keep concurrent tracers
    from both attaching to same task? What does this do for setprocattr()?

    task_lock(p);
    tracer = tracehook_tracer_task(p);
    if (tracer)
    ptsid = task_sid(tracer);
    task_unlock(p);

    Looks like it is racy.

    cpu1 (tracer) cpu2 (tracee, changing sid)
    ------------- ---------------------------
    task_lock(tracee);
    __ptrace_may_access(tracee, ATTACH);
    task_unlock(tracee);
    task_lock(tracee)
    <security check passes> tracer = tracehook_tracer_task(tracee);
    if (tracer) <-- NULL, !tracee->ptrace
    ...
    update sid w/out checking against tracer
    write_lock_irq(&tasklist_lock);
    ...
    tracee->ptrace = PT_PTRACED;
    ...
    now we are tracing task w/ a sid
    that we didn't authorize to trace

    What do you think?

    thanks,
    -chris


    \
     
     \ /
      Last update: 2009-05-07 00:51    [W:0.023 / U:89.804 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site