linux-kernel - Re: [PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs rcu_read

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:	Fri, 6 Dec 2013 15:47:35 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Paul Moore <paul@...l-moore.com>
Cc:	Stephen Smalley <sds@...ho.nsa.gov>,
	James Morris <james.l.morris@...cle.com>,
	Eric Paris <eparis@...isplace.org>,
	Evan McNabb <emcnabb@...hat.com>,
	Jan Stancek <jstancek@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selinux: selinux_setprocattr()->ptrace_parent() needs
	rcu_read_lock()

On 12/05, Paul Moore wrote:
>
> On Thursday, December 05, 2013 05:59:53 PM Oleg Nesterov wrote:
> >
> > Note: perhaps we should simply kill ptrace_parent(), it buys
> > almost nothing and it is obviously racy. Or perhaps we should
> > change it to ensure it can't wrongly return the natural parent
> > if it races with ptrace_detach.
>
> Can you elaborate on "kill ptrace_parent()"?  If the process is being traced
> we do need to fetch the tracer's task_struct for use in the SELinux access
> check at this bottom of the diff below.  If you have something better in mind
> than ptrace_parent() it would be helpful to share that ...

Sorry for confusion.

I meant that the code like

	tracer = ptrace_parent(p);
	if (tracer)
		do_something(tracer);

doesn't look better than just

	if (p->ptrace)
		do_something(p->parent);

but this is subjective of course.

And perhaps I am wrong. Because otoh the usage of ->ptrace should be
avoided outside of the core kernel code.

Mostly it annoys me because it is racy, without tasklist_lock it can
return ->real_parent (which never traced its child) if it races with
attach or detach, and I do not see a simple fix.

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/