linux-kernel - Re: [PATCH -mm 2/2] do_wait: cleanup delay_group

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 27 Nov 2007 11:51:41 +1100 (EST)
From:	James Morris <jmorris@...ei.org>
To:	Oleg Nesterov <oleg@...sign.ru>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Eric Paris <eparis@...hat.com>,
	Roland McGrath <roland@...hat.com>,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	Stephen Smalley <sds@...ho.nsa.gov>
Subject: Re: [PATCH -mm 2/2] do_wait: cleanup delay_group_leader() usage

[added LSM list]

On Fri, 23 Nov 2007, Oleg Nesterov wrote:

> eligible_child() == 2 means delay_group_leader(). With the previous patch this
> only matters for EXIT_ZOMBIE task, we can move that special check to the only
> place it is really needed.
> 
> Also, with this patch we don't skip security_task_wait() for the group leaders
> in a non-empty thread group. I don't really understand the exact semantics of
> security_task_wait(), but imho this change is a bugfix.

The semantics in terms of SELinux are that calls to wait() type calls need 
to be mediated by policy, as information can be transfered via the exit 
status.

It looks like a correct bugfix to me, but I'd be interested to see what 
others have to say.



> Also rearrange the code a bit to kill an ugly "check_continued" backdoor.
> 
> Signed-off-by: Oleg Nesterov <oleg@...sign.ru>
> 
> --- PT/kernel/exit.c~6_delay_leader	2007-11-23 20:31:21.000000000 +0300
> +++ PT/kernel/exit.c	2007-11-23 21:29:44.000000000 +0300
> @@ -1137,12 +1137,6 @@ static int eligible_child(pid_t pid, int
>  	if (((p->exit_signal != SIGCHLD) ^ ((options & __WCLONE) != 0))
>  	    && !(options & __WALL))
>  		return 0;
> -	/*
> -	 * Do not consider thread group leaders that are
> -	 * in a non-empty thread group:
> -	 */
> -	if (delay_group_leader(p))
> -		return 2;
>  
>  	err = security_task_wait(p);
>  	if (err)
> @@ -1494,10 +1488,9 @@ repeat:
>  	tsk = current;
>  	do {
>  		struct task_struct *p;
> -		int ret;
>  
>  		list_for_each_entry(p, &tsk->children, sibling) {
> -			ret = eligible_child(pid, options, p);
> +			int ret = eligible_child(pid, options, p);
>  			if (!ret)
>  				continue;
>  
> @@ -1521,19 +1514,17 @@ repeat:
>  				retval = wait_task_stopped(p,
>  						(options & WNOWAIT), infop,
>  						stat_addr, ru);
> -			} else if (p->exit_state == EXIT_ZOMBIE) {
> +			} else if (p->exit_state == EXIT_ZOMBIE &&
> +					!delay_group_leader(p)) {
>  				/*
> -				 * Eligible but we cannot release it yet:
> +				 * We don't reap group leaders with subthreads.
>  				 */
> -				if (ret == 2)
> -					goto check_continued;
>  				if (!likely(options & WEXITED))
>  					continue;
>  				retval = wait_task_zombie(p,
>  						(options & WNOWAIT), infop,
>  						stat_addr, ru);
>  			} else if (p->exit_state != EXIT_DEAD) {
> -check_continued:
>  				/*
>  				 * It's running now, so it might later
>  				 * exit, stop, or stop and then continue.
> 

-- 
James Morris
<jmorris@...ei.org>
-
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/