Re: [PATCH 3/3] ptrace: do not use task_lock() for attach

[Roland McGrath]

> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/