linux-kernel - Re: [PATCH] prctl: add PR_{SET,GET}_CHILD_SUBREAPER to allow simple process supervision

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPXgP116b7GTzjTczXWq_v8bjATXuogJs7PU7oSY-5CimU4umQ@mail.gmail.com>
Date:	Sat, 14 Jan 2012 14:59:03 +0100
From:	Kay Sievers <kay.sievers@...y.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Oleg Nesterov <oleg@...hat.com>, Valdis.Kletnieks@...edu,
	linux-kernel@...r.kernel.org,
	Lennart Poettering <lennart@...ttering.net>
Subject: Re: [PATCH] prctl: add PR_{SET,GET}_CHILD_SUBREAPER to allow simple
 process supervision

On Sat, Jan 14, 2012 at 01:35, Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Mon, 9 Jan 2012 16:07:01 +0100
> Oleg Nesterov <oleg@...hat.com> wrote:
>
>> > > +         /* find the first ancestor marked as child_subreaper */
>> > > +         for (reaper = father->real_parent;
>> > > +              reaper != &init_task;
>> > > +              reaper = reaper->real_parent) {
>> >
>> > I admit being insufficiently caffienated - does this DTRT in a PID namespace? That
>> > &init_task looks fishy to me...
>>
>> Probably this needs a comment. Initially I was confused too.
>>
>> Note that the code below checks same_thread_group(reaper, pid_ns->child_reaper),
>> this is what we need to DTRT in a PID namespace. However we still need the
>> check above, see http://marc.info/?l=linux-kernel&m=131385460420380
>
> In light of Kay's haughty silence, I did this:
>
> --- a/kernel/exit.c~prctl-add-pr_setget_child_subreaper-to-allow-simple-process-supervision-fix-fix
> +++ a/kernel/exit.c
> @@ -724,7 +724,13 @@ static struct task_struct *find_new_reap
>        } else if (father->signal->has_child_subreaper) {
>                struct task_struct *reaper;
>
> -               /* find the first ancestor marked as child_subreaper */
> +               /*
> +                * Find the first ancestor marked as child_subreaper.
> +                * Note that the code below checks same_thread_group(reaper,
> +                * pid_ns->child_reaper).  This is what we need to DTRT in a
> +                * PID namespace. However we still need the check above, see
> +                * http://marc.info/?l=linux-kernel&m=131385460420380
> +                */
>                for (reaper = father->real_parent;
>                     reaper != &init_task;
>                     reaper = reaper->real_parent) {

Oleg and I got nowhere really, discussing if we should switch the
check in the for() parameters and the one inside the loop; so we just
decided to keep it the way it is, and did not reply any further. It
wasn't that we didn't care, we just couldn't decide. Sorry for that.

The comment indeed sounds good to have. Thanks for adding this.

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