lkml.org 
[lkml]   [2006]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] fix de_thread() vs do_coredump() deadlock
    On 04/09, Roland McGrath wrote:
    >
    > [PATCH] Fix race between exec and fatal signals

    I'll try to study this patch carefully tomorrow, but now I have
    the feeling it is not right (probably my misunderstanding after
    the quick reading).

    > --- a/fs/exec.c
    > +++ b/fs/exec.c
    > @@ -606,15 +606,16 @@ static int de_thread(struct task_struct
    >
    > ... [snip] ...
    >
    > - zap_other_threads(current);
    > + zap_other_threads(current, SIGNAL_GROUP_EXEC);
    >
    > ... [snip] ...
    >
    > -void zap_other_threads(struct task_struct *p)
    > +void zap_other_threads(struct task_struct *p, int flag)
    > {
    > struct task_struct *t;
    >
    > - p->signal->flags = SIGNAL_GROUP_EXIT;
    > + if (unlikely(p->signal->flags & SIGNAL_GROUP_EXEC)) {
    > + /*
    > + * We are cancelling an exec that is in progress, to let
    > + * the thread group die instead. We need to wake the
    > + * exec'ing thread up from uninterruptible wait.
    > + */
    > + BUG_ON(flag != SIGNAL_GROUP_EXIT);
    > + t = p->signal->group_exit_task;
    > + p->signal->group_exit_task = NULL;
    > + p->signal->notify_count = 0;
    > + wake_up_process(t);
    > + }
    > +
    > + p->signal->flags = flag;
    > p->signal->group_stop_count = 0;

    So, de_thread() sets SIGNAL_GROUP_EXEC and sends SIGKILL to other thereads.

    Sub-thread receives the signal, and calls get_signal_to_deliver->do_group_exit.
    do_group_exit() calls zap_other_threads(SIGNAL_GROUP_EXIT) because there is no
    SIGNAL_GROUP_EXIT set. zap_other_threads() notices SIGNAL_GROUP_EXEC, wakes up
    execer, and changes ->signal->flags to SIGNAL_GROUP_EXIT.

    de_thread() re-locks sighand, sees !SIGNAL_GROUP_EXEC and goes to 'dying:'.

    No?

    Another problem. de_thread() sets '->group_exit_task = current' _only_ if
    'atomic_read(&sig->count) > count', so wake_up_process(->group_exit_task)
    in zap_other_threads() is unsafe.

    Oleg.

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

    \
     
     \ /
      Last update: 2006-04-10 15:49    [W:2.762 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site