linux-kernel - eligible_child() && __WCLONE && task_detached() (Was: mmotm 2009-08-24-16-24 uploaded)

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: <20090828171724.GA17445@redhat.com>
Date:	Fri, 28 Aug 2009 19:17:24 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Roland McGrath <roland@...hat.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: eligible_child() && __WCLONE && task_detached() (Was: mmotm
	2009-08-24-16-24 uploaded)

eligible_child:

	/* Wait for all children (clone and not) if __WALL is set;
	 * otherwise, wait for clone children *only* if __WCLONE is
	 * set; otherwise, wait for non-clone children *only*.  (Note:
	 * A "clone" child here is one that reports to its parent
	 * using a signal other than SIGCHLD.) */

	if (((p->exit_signal != SIGCHLD) ^ !!(wo->wo_flags & __WCLONE))
	    && !(wo->wo_flags & __WALL))
		return 0;

I just can't understand what is the supposed behaviour when p is
sub-thread and p->exit_signal == -1.

This only matters when the caller is ptracer, and p is tracee. In
this case ptracer should use __WCLONE or __WALL, this doesn't look
very logical. Firstly, the comment says

	* A "clone" child here is one that reports to its parent
	* using a signal other than SIGCHLD.

but, unless ptraced, sub-thread reports nothing to ->parent.

IOW, perhaps this check should be

	if (!task_detached(p) && !(wo->wo_flags & __WALL) &&
	    (p->exit_signal != SIGCHLD) ^ !!(wo->wo_flags & __WCLONE))
		return 0;

?

When task_detached(p) == T, "p->exit_signal != SIGCHLD" looks like a
false positive to me. Because -1 is not a siganl, this is a marker
which indicates the deatached task - sub-thread or EXIT_DEAD. It _seems_
to me this check was added when threads were processes, and it was
possible to wait/reap a thread, not process.

In short. If ptracer calls wait4(ptraced_sub_thread), is it really
supposed it must use __WCLONE || __WALL?

(this check also breaks child_wait_callback() logic in
  do_wait-wakeup-optimization-change-__wake_up_parent-to-use-filtered-wakeup.patch
  but we can fix this in many ways. Just I am not sure _what_ should be
  fixed)

Oleg.

On 08/27, Oleg Nesterov wrote:
>
> On 08/27, KAMEZAWA Hiroyuki wrote:
> >
> > On Thu, 27 Aug 2009 12:08:46 +0200
> > Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > >
> > > OK, I seem to understand what happens. Could you try the patch below?
> > >
> >
> > worked.
>
> Thanks. I need to think a bit, then I send the fix.
>
> > IMHO, it's necessary to "wake up parent with -ECHILD if all children dies"
>
> Of course! It was supposed to do. More precisely, we should wake up when
> any child which cuould be interesting to ->parent dies. No need to check
> "all children died" case specially. If parent sleeps on ->wait_chldexit
> there must be at least on eligible child.
>
> The problem is, do_notify_parent() changes ->exit_signal _before_ it calls
> __wake_up_parent(). This changes the result of eligible_child().
>
> 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/