linux-kernel - Re: [PATCH 27/43] userns: Use uid_eq gid_eq helpers when comparing kuids and kgids in the vfs

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 20 Apr 2012 17:58:35 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	"Serge E. Hallyn" <serge@...lyn.com>
Cc:	linux-kernel@...r.kernel.org,
	Linux Containers <containers@...ts.linux-foundation.org>,
	Cyrill Gorcunov <gorcunov@...nvz.org>,
	linux-security-module@...r.kernel.org,
	Al Viro <viro@...IV.linux.org.uk>,
	linux-fsdevel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 27/43] userns: Use uid_eq gid_eq helpers when comparing kuids and kgids in the vfs

"Serge E. Hallyn" <serge@...lyn.com> writes:

> Quoting Eric W. Beiderman (ebiederm@...ssion.com):
>> From: Eric W. Biederman <ebiederm@...ssion.com>
>> 
>> Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>
>> ---
>>  fs/attr.c                |    8 ++++----
>>  fs/exec.c                |   10 +++++-----
>>  fs/fcntl.c               |    6 +++---
>>  fs/ioprio.c              |    4 ++--
>>  fs/locks.c               |    2 +-
>>  fs/namei.c               |    8 ++++----
>>  include/linux/quotaops.h |    4 ++--
>>  7 files changed, 21 insertions(+), 21 deletions(-)
>> 

>> @@ -2120,7 +2120,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
>>  	if (__get_dumpable(cprm.mm_flags) == 2) {
>>  		/* Setuid core dump mode */
>>  		flag = O_EXCL;		/* Stop rewrite attacks */
>> -		cred->fsuid = 0;	/* Dump root private */
>> +		cred->fsuid = GLOBAL_ROOT_UID;	/* Dump root private */
>
> Sorry, one more - can this be the per-ns root uid?  The coredumps should
> be ok to belong to privileged users in the namespace right?

I'm not certain it was clear when you were looking at this that
this is about dumping core from suid applications, not normal
applications. 

 Looking at the code in commoncap and commit_creds it looks like it is a
bug that we don't call set_dumpable(new, suid_dumpable) in common cap
when we use file capabilities.  I might be wrong but I think we escape
the test in commit_creds in that case.

Having thought about it we can make this per namespace but not in
this patch.

Things that I see as missing.
- We likely need to make the suid_dumpable sysctl per namespace.
  There is a prctl so it is already per process.
- We would need to capture the user_namespace at mm creation time,
  during exec, so we know which root user we could use.

  By it's nature we know an mm can't escape a user namespace so the
  user namespace an mm is created in will have a root user we can
  dump core as.

I was wondering if we could relax this to a uid captured at mm creation
time (and certainly we can capture the root user), but there are enough
weird cases I don't think it is possible to safely allow anything more
relaxed that the root of the user_namespace that created the mm.

I don't believe we can't use the user_namespace of current because the
application may have been suid and then cloned a new user namespace
keeping the mm or perhaps just the uid/euid split.

So in short it is doable but a little tricky so it doesn't belong in
a patch with a bunch of boring and safe conversions.

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