On Fri, 2 Aug 2002, Dave McCracken wrote:
>
> This patch provides the ability to share credentials (uid, gid,
> capabilities) between tasks, using a clone() flag.
I worry about the lack of locking here.
Maybe it's the right thing to do, I don't really know.
But I _know_, for example, that this is just a horrid security hole the
way it is now - the execve() path doesn't create a unique "cred"
structure, so if you execve() a suid binary from a CLONE_CRED thread, the
other threads get the suid'ness and can do whatever they want.
At the very least, it should disallow suid exec's when
atomic_read(¤t->cred->count) > 1
which is the same approach we do wrt other shared state (ie disallow a
CLONE_FILES thing from doing a suid execve etc).
The alternative is to just allocate a new cred structure on execve.
As-is this patch is way way too dangerous. You can trivially create a root
hole by doing
if (!clone(CLONE_CRED)) {
execve("su");
exit(1);
}
..this thread now also got root..
> There is no lock around the credential accesses, but from my analysis none
> is needed.
You may be right. I don't see any huge reason for it, but see above on
other fundamental problems.
Linus
--On Friday, August 02, 2002 10:22:54 AM -0700 Linus Torvalds
<[email protected]> wrote:
> But I _know_, for example, that this is just a horrid security hole the
> way it is now - the execve() path doesn't create a unique "cred"
> structure, so if you execve() a suid binary from a CLONE_CRED thread, the
> other threads get the suid'ness and can do whatever they want.
You are entirely correct. It was an oversight on my part. execve() should
always unshare the cred structure. I'll work up a fixed version.
Dave McCracken
======================================================================
Dave McCracken IBM Linux Base Kernel Team 1-512-838-3059
[email protected] T/L 678-3059