linux-kernel - Re: [PATCH 0/4] Rebase device

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: <20130514150539.GA26090@sergelap>
Date:	Tue, 14 May 2013 10:05:39 -0500
From:	Serge Hallyn <serge.hallyn@...ntu.com>
To:	Aristeu Rozanski <aris@...hat.com>
Cc:	linux-kernel@...r.kernel.org,
	"Eric W. Biederman" <ebiederm@...ssion.com>, amorgan@...gelap,
	cgroups@...r.kernel.org
Subject: Re: [PATCH 0/4] Rebase device_cgroup v2 patchset

Hi,

so now that the device cgroup properly respects hierarchy, not allowing
a cgroup to be given greater permission than its parent, should we consider
relaxing the capability checks?

There are two capable(CAP_SYS_ADMIN) checks in deice_cgroup.c: one in
devcgroup_can_attach() to protect changing another task's cgroup, and
one in devcgroup_update_access() to protect writes to the devices.allow
and devices.deny files.

I think the first should be changed to a check for ns_capable() to
the victim's user_ns.  Something like 

--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -70,10 +70,16 @@ static int devcgroup_can_attach(struct cgroup *new_cgrp,
                                struct cgroup_taskset *set)
 {
        struct task_struct *task = cgroup_taskset_first(set);
+       struct user_namespace *ns;
+       int ret = -EPERM;

-       if (current != task && !capable(CAP_SYS_ADMIN))
-               return -EPERM;
-       return 0;
+       if (current == task)
+               return 0;
+
+       ns = userns_get(task);;
+       ret = ns_capable(ns, CAP_SYS_ADMIN) ? 0 : -EPERM;
+       put_user_ns(ns);
+       return ret;
 }

 /*

For the second, the hierarchy support should let us ignore concerns
about unprivileged users escalating privilege, but I'm trying to decide
whether we  need to worry about the sendmail capability class of bugs.
My sense is actually the answer is no, and we can drop the capable()
check altogether.  The reason is that while userspace frequently doesn't
properly handle a failing system call due to unexpected lack of partial
privilege, I wouldn't expect any setuid root program to ignore failure
to open or mknod a device file (and proceed into a bad failure mode).
Does this sound rasonable, or a recipe for disaster?

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