linux-kernel - Re: [PATCH 3/3] ptrace: do not use task

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: <20090507011716.BA522FC39E@magilla.sf.frob.com>
Date:	Wed,  6 May 2009 18:17:16 -0700 (PDT)
From:	Roland McGrath <roland@...hat.com>
To:	James Morris <jmorris@...ei.org>
Cc:	Chris Wright <chrisw@...s-sol.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	Eric Paris <eparis@...hat.com>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	David Howells <dhowells@...hat.com>
Subject: Re: [PATCH 3/3] ptrace: do not use task_lock() for attach

> As far as I can tell, yes.
> 
> (Added David Howells and security folk to the cc -- please make sure at 
> least that the LSM list is cc'd when changing code which affects LSM 
> modules).

Good catch, Chris and Oleg!  This one is yet another dhowells blue plate
special, deeply subtle change buried inside the ginormous commit d84f4f9. ;-}
He even mentioned this one in the log:

 (a) selinux_setprocattr() no longer does its check for whether the
	 current ptracer can access processes with the new SID inside the lock
	 that covers getting the ptracer's SID.  Whilst this lock ensures that
	 the check is done with the ptracer pinned, the result is only valid
	 until the lock is released, so there's no point doing it inside the
	 lock.

Before d84f4f9, the extraction, avc check, and SID switch were all under
task_lock().  What David's comment ignores is that "the lock that covers
getting the ptracer's SID" (i.e. task_lock) is also the lock that excludes
ptrace attempts, with their security checks against the (old or new) SID.
i.e.:

			if (!error)
				tsec->sid = sid;
			task_unlock(p);

That ensured that ptrace_attach/ptrace_traceme would be seen to atomically
check the bits that affect the SELinux ptrace controls and change the bits
that affect "if (tracer)".

Indeed, cred_exec_mutex is the equivalent lock for that post-d84f4f9.

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/