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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
