linux-kernel - Re: [PATCH 1/4] forget_original_parent: split out the un-ptrace part

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: <20090225003408.1DA81FC380@magilla.sf.frob.com>
Date:	Tue, 24 Feb 2009 16:34:08 -0800 (PST)
From:	Roland McGrath <roland@...hat.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Metzger, Markus T" <markus.t.metzger@...el.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] forget_original_parent: split out the un-ptrace
	part

> But from the _pure theoretical_ pov, it is not correct to assume that
> list_empty(&tracer->ptraced) == T means that current can not be used
> somehow as tracee->parent. Another subthread can release a dead tracee.

I don't follow how that's relevant.  If list_empty(), then it was empty or
is becoming empty.  It can't then become nonempty again (because the thread
doing the check is the only one that adds to that list).  That's all we're
assuming.

> For example, list_empty(&tracer->ptraced) == T doesn't mean that the
> STOREs to this task_struct are finished, list_del_init(->ptrace_entry)
> can still be in progress.

Sure, but so what?  The check is to verify that some new list_del* (and
related cleanup work, of course) doesn't need to be *started*.

> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -534,7 +534,7 @@ repeat:
> >  		 * Set the ptrace bit in the process ptrace flags.
> >  		 * Then link us on our parent's ptraced list.
> >  		 */
> > -		if (!ret) {
> > +		if (!ret && !(current->real_parent->flags & PF_EXITING)) {
> >  			current->ptrace |= PT_PTRACED;
> 
> Yes sure.
> 
> But this means exit_ptrace() must always take tasklist, otherwise we
> don't have the necessary barriers.

Really?

	exit_signals(tsk);  /* sets PF_EXITING */
	/*
	 * tsk->flags are checked in the futex code to protect against
	 * an exiting task cleaning up the robust pi futexes.
	 */
	smp_mb();

This is an exactly analogous use, isn't it?  So exit_ptrace() just has to
follow this same existing barrier.  Right?


Thanks,
Roland

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