linux-kernel - Re: [PATCH UPDATED 8/9] ptrace: move JOBCTL_TRAPPING wait to wait(2) and ptrace_check

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]

Date:	Mon, 16 May 2011 14:11:42 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	jan.kratochvil@...hat.com, vda.linux@...glemail.com,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	akpm@...ux-foundation.org, indan@....nu, bdonlan@...il.com
Subject: Re: [PATCH UPDATED 8/9] ptrace: move JOBCTL_TRAPPING wait to
	wait(2) and ptrace_check_attach()

On 05/14, Tejun Heo wrote:
>
> @@ -1409,15 +1409,29 @@ static int wait_task_stopped(struct wait
>  	if (!ptrace && !(wo->wo_flags & WUNTRACED))
>  		return 0;
>
> -	if (!task_stopped_code(p, ptrace))
> +	/*
> +	 * For ptrace waits, we can't reliably check whether wait condition
> +	 * exists without grabbing siglock due to JOBCTL_TRAPPING
> +	 * transitions.  A task might be temporarily in TASK_RUNNING while
> +	 * trapping which should be transparent to the ptracer.
> +	 *
> +	 * Note that we can avoid unconditionally grabbing siglock by
> +	 * wrapping TRAPPING test with two rmb's; however, let's stick with
> +	 * simpler implementation for now.
> +	 */
> +	if (!ptrace && !(p->signal->flags & SIGNAL_STOP_STOPPED))
>  		return 0;
>
>  	exit_code = 0;
>  	spin_lock_irq(&p->sighand->siglock);
>
>  	p_code = task_stopped_code(p, ptrace);
> -	if (unlikely(!p_code))
> +	if (unlikely(!p_code)) {
> +		/* if trapping, wait for it and restart the whole process */
> +		if (ptrace && ptrace_wait_trapping(p))
> +			return restart_syscall();

Hmm. I didn't even know we have restart_syscall()... It is a bit fragile,
it assumes recalc_sigpending() is not possible during return from syscall.
In particular this means recalc_sigpending() must not be called in irq.
OK, this seems to be true.

Anyway, restart_syscall() is not right for do_wait(), especially with the
next patch. If the caller was woken by the real signal which has a handler,
we should not restart without SA_RESTART.

It is very hard to review this series. Without the further changes, it is
not clear why do we need these preparations. IIUC, ptrace_wait_trapping()
is only needed because we are going to re-trap. Otherwise we could always
wait in ptrace_attach() afaics.

I am still worried we are loosing the tight control over JOBCTL_TRAPPING.
6/9 contributes to this too.

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/