linux-kernel - Re: + do_wait-wakeup-optimization.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-next>] [day] [month] [year] [list]

Date:	Sun, 23 Nov 2008 22:39:29 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	akpm@...ux-foundation.org
Cc:	linux-kernel@...r.kernel.org, roland@...hat.com, mingo@...e.hu,
	rnalumasu@...il.com
Subject: Re: + do_wait-wakeup-optimization.patch added to -mm tree

> From: Roland McGrath <roland@...hat.com>
>
> +static int needs_wakeup(struct task_struct *task, struct do_wait_queue_entry *w)
> +{
> +	if ((w->options & __WNOTHREAD) && task->parent != w->wq.private)
> +		return 0;
> +
> +	if (eligible_child(w->type, w->pid, w->options,
> +			   task, task->exit_signal))
> +		return 1;
> +
> +	if (thread_group_leader(task)) {
> +		/*
> +		 * In a group leader, do_notify_parent() may have
> +		 * just reset task->exit_signal because SIGCHLD was
> +		 * ignored, but that doesn't prevent the wakeup.
> +		 */
> +		if (!task_detached(task) ||
> +		    !eligible_child(w->type, w->pid, w->options,
> +				    task, SIGCHLD))
> +			return 0;
> +	} else {
> +		/*
> +		 * In a non-leader, this might be the release_task()
> +		 * case, where it's the leader rather than task
> +		 * whose parent is being woken.
> +		 */
> +		if (!eligible_child(w->type, w->pid, w->options,
> +				    task->group_leader,
> +				    task_detached(task->group_leader) ?
> +				    SIGCHLD : task->group_leader->exit_signal))
> +			return 0;
> +	}
> +
> +	return 1;
> +}

Unless I missed something, this is not right.

This "task" is current, iow it is the caller of do_notify_parent(). Sometime
it is OK (release_task, exit_notify), but in general not, afaics.

Let's suppose the ptracer finds the EXIT_ZOMBIE tracee and notifies its
->real_parent which sleeps in do_wait(). In that case the usage of
eligible_child(task == ptracer) above is bogus, and checking for
group_leader is not rifgt too.

> +static int do_wait_wake_function(wait_queue_t *curr, unsigned mode, int sync,
> +				 void *key)
> +{
> +	struct task_struct *task = current;

I think we can fix (and simplify) this code if we change __wake_up_parent(),
it should call __wake_up(key => p), so we can do

	struct task_struct *task = key;

> +	if (!needs_wakeup(task, w))
> +		return 0;
> +
> +	return default_wake_function(curr, mode, sync, key);

perhaps autoremove_wake_function() makes more sense.

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/