linux-kernel - Re: ptrace fixes for 3.2

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: <20120103163023.GA31746@google.com>
Date:	Tue, 3 Jan 2012 08:30:23 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Denys Vlasenko <vda.linux@...glemail.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	linux-kernel@...r.kernel.org,
	Łukasz Michalik <lmi@....uni.wroc.pl>,
	"Dmitry V. Levin" <ldv@...linux.org>
Subject: Re: ptrace fixes for 3.2

Hello, Oleg.

On Tue, Jan 03, 2012 at 04:44:04PM +0100, Oleg Nesterov wrote:
> It fails because ->real_parent sees its child in EXIT_DEAD state
> while the tracer is going to change the state back to EXIT_ZOMBIE
> in wait_task_zombie().

Argh.... EXIT_ZOMBIE -> DEAD -> ZOMBIE dancing in wait_task_zombie()
is just nasty.  Didn't realize it was doing that.  :(

> The offending commit is 823b018e which moved the EXIT_DEAD check,
> but in fact we should not blame it. The original code was not
> correct as well because it didn't take ptrace_reparented() into
> account and because we can't really trust ->ptrace.
> 
> This patch adds the additional check to close this particular
> race but it doesn't solve the whole problem. We simply can't
> rely on ->ptrace in this case, it can be cleared if the tracer
> is multithreaded by the exiting ->parent.

I'm not following this part.  Can you please explain it in a bit more
detail?

> I think we should kill EXIT_DEAD altogether, we should always
> remove the soon-to-be-reaped child from ->children or at least
> we should never do the DEAD->ZOMBIE transition. But this is too
> complex for 3.2.

Agreed.  Removing the reverse transition shouldn't be too difficult
and can be done without affecting fast non-ptrace path.  ie. if the
child is ptraced, drop readlock, grab writelock, recheck, buffer
states to copy out to userland, detach and transit to DEAD if
necessary.

> Also, I think wait_consider_task() needs more fixes. I do not
> think we should clear ->notask_error without WEXITED in this
> case, but this is what we do in the EXIT_ZOMBIE case.

Hmmm... I'm not sure about that.  Why do you think so?

> Reported-by: Denys Vlasenko <vda.linux@...glemail.com>
> Cc: v3.0.. <stable@...nel.org>
> Signed-off-by: Oleg Nesterov <oleg@...hat.com>

Anyways, the fix looks good to me.

 Acked-by: Tejun Heo <tj@...nel.org>

Thank you.

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