linux-kernel - Re: + prctl-add-pr_setget_child_reaper-to-allow-simple-process-supervision .patch added to -mm tree

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: <20110818184857.GA12094@redhat.com>
Date:	Thu, 18 Aug 2011 20:48:57 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Kay Sievers <kay.sievers@...y.org>
Cc:	Lennart Poettering <mzxreary@...inter.de>,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	linux-man@...r.kernel.org, roland@...k.frob.com,
	torvalds@...ux-foundation.org
Subject: Re: +
	prctl-add-pr_setget_child_reaper-to-allow-simple-process-supervision
	.patch added to -mm tree

Hello Kay,

I need to go away, I'll read this patch (and the whole email) tomorrow.

Just a quick note right now,

On 08/18, Kay Sievers wrote:
>
>  static struct task_struct *find_new_reaper(struct task_struct *father)
>  	__releases(&tasklist_lock)
> @@ -724,6 +725,19 @@ static struct task_struct *find_new_reap
>  		 * forget_original_parent() must move them somewhere.
>  		 */
>  		pid_ns->child_reaper = init_pid_ns.child_reaper;
> +	} else {
> +		/* find the first ancestor which is marked as child_reaper */
> +		for (thread = father->real_parent;
> +		     thread != thread->real_parent;
> +		     thread = thread->real_parent) {
> +			if (thread == pid_ns->child_reaper)
> +				break;
> +			if (!thread->signal->child_reaper)
> +				continue;
> +			if (thread->flags & PF_EXITING)
> +				continue;
> +			return thread;

No, this doesn't look right.

This code should do something like

		for (reaper = father->real_parent;
		     !same_thread_group(reaper, pid_ns->child_reaper);
		     reaper = reaper->real_parent) {
			if (!signal->child_reaper)
				continue;

			if (there is a !PF_EXITING thread)
				return thread;
		}

And I forgot to mention, could you please-please rename child_reaper?
Say, is_child_reaper or is_sub_reaper. Or whatever. I do not really
care about the naming. But I use grep very often, and personally I
dislike the task->child_reaper/signal->child_reaper confusion.

Oleg.

--
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/