* Re: [PATCH v2 0/7] Smack namespace
From: Lukasz Pawelczyk @ 2015-05-27 10:13 UTC (permalink / raw)
To: Eric W. Biederman, Casey Schaufler
Cc: David S. Miller, Kirill A. Shutemov, Serge E. Hallyn, Al Viro,
Alexey Dobriyan, Andrew Morton, Andy Lutomirski, David Howells,
Fabian Frederick, Greg KH, James Morris, Jeff Layton, Jingoo Han,
Joe Perches, John Johansen, Jonathan Corbet, Kees Cook,
Mauro Carvalho Chehab, Miklos Szeredi, Oleg Nesterov, Paul Moore,
Stephen Smalley, Tetsuo Handa
In-Reply-To: <87h9qyww4h.fsf@x220.int.ebiederm.org>
On wto, 2015-05-26 at 22:13 -0500, Eric W. Biederman wrote:
> Lukasz Pawelczyk <l.pawelczyk@samsung.com> writes:
>
> > Hello,
> >
> > Some time ago I sent a Smack namespace documentation and a preliminary
> > LSM namespace for RFC. I've been suggested that there shouldn't be a
> > separate LSM namespace and that it should live within user namespace.
> > And this version does. This is a complete set of patches required for
> > Smack namespace.
> >
> > This was designed with a collaboration of Smack maintainer Casey
> > Schaufler.
> >
> > Smack namespace have been implemented using user namespace hooks added
> > by one of the patches. To put some context to it I paste here a
> > documentation on what Smack namespace wants to achieve.
> >
> > LSM hooks themselves are documented in the security.h file inside the
> > patch.
> >
> > The patches are based on:
> > https://github.com/cschaufler/smack-next/tree/smack-for-4.2-stacked
> >
> > The tree with them is avaiable here:
> > https://github.com/Havner/smack-namespace/tree/smack-namespace-for-4.2-stacked-v2
> >
> > Changes from v1:
> > - "kernel/exit.c: make sure current's nsproxy != NULL while checking
> > caps" patch has been dropped
> > - fixed the title of the user_ns operations patch
>
> A have not completed a review I don't understand smack well enough to
> answer some questions but I don't think I like the approach this patch
> takes to get things done.
>
> I am flattered that you are using a mapping as I did with uid map and
> gid map. Unfortunately I do not believe your mapping achieves what my
> mapping of uids and gids achieved.
>
> A technical goal is to give people the tools so that a sysadmin can set
> up a system, can grant people subids and subgids, and then the user can
> proceed to do what they need to do.
This is exactly the case with Smack namespace.
> In particular there should be
> little to no need to keep pestering the system administrator for more
> identifiers.
This all depends on the use case. When you create a new namespace for a
particular purpose you could know what labels will be required there and
map them upfront. Adding new mappings is for a use case that a container
might have new software installed that requires new labels for it. You
don't create/import new labels on a normal basis all the time.
> The flip side of that was that the mapping would ensure all of the
> existing permissions checks would work as expected, and the checks in
> the kernel could be converted without much trouble.
Again, this is exactly the case with Smack namespace. There are
additional checks in place of course, but the rules are clear about
how that works.
> Ranges of ids were choosen because they allow for a lot of possible ways
> of using uids and gids in containers, are comparitively easy to
> administer, are very fast to use, and don't need large data structures.
This is Smack specific. The same way we could compare Smack with DAC
(even without talking about namespaces). Every new UID is just it, a
number. Every new label requires allocation and adds to the list.
I had an idea about mapping ranges of labels for a container by using
prefixes but this was dropped by Casey. Labels (with an exception of the
built-in ones) should not have any special meaning. This is Smack
design.
About performance, as it was originally with Smack. The list of labels
was a simple list. When the performance was not enough hashing was
added. I'd take the same approach here. If some use cases will require
so many mappings that this becomes a problem we will deal with it. I
don't want to complicate the patches now and I this was Casey's opinion
as well.
> With a discreet mapping of labels I have the premonition that we now
> have a large data structure that, is not as flexible in to use,
> is comparatively slow and appears to require an interaction with the
> system administrator for every label you use in a container.
Again, DAC vs Smack philosophy. The same way a new label appears in init
namespace (e.g. with a file/inode). Without a new rule you won't have an
access to it. This requires administrator intervention as well. This is
MAC after all.
> As part of that there is added a void *security pointer in the user
> namespace to apparently hang off anything anyone would like to use.
> Connected to that are hooks that have failure codes (presumably memory
> allocation failures), but the semantics are not clear. My gut feel is
> that I would rather extend struct user_namespace to hold the smack label
> mapping table and remove all of the error codes because they would then
> be unnecessary.
How is this different then filling a void *security pointer for other
kernel objects (tasks, indodes, ipc, etc). The allocations can fail as
well. I don't see how the semantics are different here.
So you would have me a Smack generic pointer in user_namespace without
any LSM abstraction? How does that cope with LSM philosophy and recently
introduced (initial) LSM stacking?
> I am also concerned that several of the operations assume that setns
> and the like are normally privileged operations and so require the
> ability to perform other privileged operations. Given that in the right
> circumstances setns is not privileged that seems like a semantics
> mismatch.
Sorry, I don't exactly know what you mean. setns remains unprivileged
operation here. The only limit is to refuse setns when adding a process
to a container would break Smack namespace rules. In theory this
limitation could be removed (I think), because a process with an
unmapped label would not be able to do anything in the namespace. This
is mostly for convenience.
> Or in short my gut feel says the semantics of this change are close to
> something that would be very useful, but the details make this patchset
> far less useful, usable and comprehensible than it should be.
This isn't much to go on for me unfortunately.
Anyway, thank you for your insight, and taking the time.
>
> Eric
>
>
> > ===================================================================
> >
> > --- What is a Smack namespace ---
> >
> > Smack namespace was developed to make it possible for Smack to work
> > nicely with Linux containers where there is a full operating system
> > with its own init inside the namespace. Such a system working with
> > Smack expects to have at least partially working SMACK_MAC_ADMIN to be
> > able to change labels of processes and files. This is required to be
> > able to securely start applications under the control of Smack and
> > manage their access rights.
> >
> > It was implemented using new LSM hooks added to the user namespace
> > that were developed together with Smack namespace.
> >
> >
> > --- Design ideas ---
> >
> > "Smack namespace" is rather "Smack labels namespace" as not the whole
> > MAC is namespaced, only the labels. There is a great analogy between
> > Smack labels namespace and the user namespace part that remaps UIDs.
> >
> > The idea is to create a map of labels for a namespace so the namespace
> > is only allowed to use those labels. Smack rules are always the same
> > as in the init namespace (limited only by what labels are mapped) and
> > cannot be manipulated from the child namespace. The map is actually
> > only for labels' names. The underlying structures for labels remain
> > the same. The filesystem also stores the "unmapped" labels from the
> > init namespace.
> >
> > Let's say we have those labels in the init namespace:
> > label1
> > label2
> > label3
> >
> > and those rules:
> > label1 label2 rwx
> > label1 label3 rwx
> > label2 label3 rwx
> >
> > We create a map for a namespace:
> > label1 -> mapped1
> > label2 -> mapped2
> >
> > This means that 'label3' is completely invisible in the namespace. As if
> > it didn't exist. All the rules that include it are ignored.
> >
> > Effectively in the namespace we have only one rule:
> > mapped1 mapped2 rwx
> >
> > Which in reality is:
> > label1 label2 rwx
> >
> > All requests to access an object with a 'label3' will be denied. If it
> > ever comes to a situation where 'label3' would have to be printed
> > (e.g. reading an exec or mmap label from a file to which we have
> > access) then huh sign '?' will be printed instead.
> >
> > All the operations in the namespace on the remaining labels will have
> > to be performed using their mapped names. Things like changing own
> > process's label, changing filesystem label. Labels will also be
> > printed with their mapped names.
> >
> > You cannot import new labels in a namespace. Every operation that
> > would do so in an init namespace will return an error in the child
> > namespace. You cannot assign an unmapped or not existing label to an
> > object. You can only operate on labels that have been explicitly
> > mapped.
> >
> >
> > --- Capabilities ---
> >
> > Enabling Smack related capabilities (CAP_MAC_ADMIN and
> > CAP_MAC_OVERRIDE) is main goal of Smack namespace, so it can work
> > properly in the container. And those capabilities do work to some
> > extent. In several places where capabilities are checked compatibility
> > with Smack namespace has been introduced. Capabilities are of course
> > limited to operate only on mapped labels.
> >
> > CAP_MAC_OVERRIDE works fully, will allow you to ignore Smack access
> > rules, but only between objects that have labels mapped. So in the
> > example above having this CAP will allow e.g. label2 to write to
> > label1, but will not allow any access to label3.
> >
> > With CAP_MAC_ADMIN the following operations has been allowed inside
> > the namespace:
> > - setting and removing xattr on files, including the security.* ones
> > - setting process's own label (/proc/self/attr/current)
> > - mounting in a privileged Smack mode, which means one can specify
> > additional mount options like: smackfsdef, smackfsfloor etc.
> >
> > Again this is also allowed only on the mapped labels. Labels on the
> > filesystem will be stored in unmapped form so they are preserved
> > through reboots.
> >
> > Such a namespace construct allows e.g. systemd (with Smack support)
> > working in a container to assign labels properly to daemons and other
> > processes.
> >
> >
> > --- Usage ---
> >
> > Smack namespace is written using LSM hooks inside user namespace. That
> > means it's connected to it.
> >
> > To create a new Smack namespace you need to unshare() user namespace
> > as usual. If that is all you do though, than there is no difference to
> > what is now. To activate the Smack namespace you need to fill the
> > labels' map. It is in a file /proc/$PID/smack_map.
> >
> > By default the map is empty and Smack namespaces are inactive (labels
> > are taken directly from a parent namespace). It also means that the
> > Smack capabilities will be inactive. After you fill the map it starts
> > to take effect in the namespace and Smack capabilities (only on mapped
> > labels) start to work.
> >
> > Due to the way Smack works only CAP_MAC_ADMIN from the parent
> > namespace (init_user_ns for now, see the "Current limitations" below)
> > is allowed to fill the map. That means that an unprivileged user is
> > still allowed to create the user namespace but it will not be able to
> > fill the labels' map (activate Smack namespace). An administrator
> > intervention is required.
> >
> > The attr_map write format is:
> > unmapped_label mapped_label
> >
> > When reading the file it shows an active map for a namespace the
> > process in question is in in the format:
> > unmapped_label -> mapped_label
> >
> > If the smack_map file is empty it means the namespace is not mapped
> > and Smack namespace is inactive (no mappings, MAC related capabilities
> > behave as they did before, meaning they are active only in
> > init_user_ns). For init_user_ns the map will always be empty.
> >
> > Writing to the map file is not disabled after the first write as it is
> > in uid_map. For Smack we have no means to map ranges of labels, hence
> > it can really be advantageous to be able to expand the map later
> > on. But you can only add to the map. You cannot remove already mapped
> > labels. You cannot change the already existing mappings. Also mappings
> > has to be 1-1. All requests to create a map where either the unmapped
> > or the mapped label already exists in the map will be denied.
> >
> > setns() with Smack namespace active has an additional check that the
> > label of a process that is calling setns() has to be already mapped in
> > the target Smack namespace for the call to succeed.
> >
> > --- Special labels ---
> >
> > Smack is using some special labels that have built-in rules. Things
> > like floor '_', dash '^', star '*', etc. Those labels are not
> > automatically mapped to the namespace. Moreover, you can choose to map
> > a different label from the init namespace to behave e.g. like floor
> > inside the namespace.
> >
> > Let's say we have no rules and those labels in the init namespace:
> > _
> > floor_to_be
> > label
> >
> > Both 'label' and 'floor_to_be' can read objects with '_'. But they
> > have no access rights to each other.
> >
> > Now let's create a map like this:
> > _ ordinary_label
> > floor_to_be _
> > label mapped
> >
> > Right now label 'mapped' can read label '_' which means that
> > effectively inside this namespace label 'label' has gained read access
> > to the 'floor_to_be'. The label 'ordinary_label' is exactly it, an
> > ordinary label that the built-in rules no longer apply to inside the
> > namespace.
> >
> > To sum up, special labels in the namespace behave the same as in the
> > init namespace. Not the original special labels though, but the ones
> > we map to specials. This is the only case where a namespace can have
> > access rights the init namespace does not have (like the 'label' to
> > 'floor_to_be' in the example above).
> >
> > Of course mappings like these are perfectly legal:
> > _ _
> > * *
> > ^ ^
> >
> >
> > --- Current limitations ---
> >
> > The Smack namespace is not hierarchical yet. It is currently not
> > possible to fill a smack_map of a nested user namespace (you can still
> > create nested user namespace, it will just inherit its parent's map
> > and won't have active Smack capabilities). When hierarchy will be
> > implemented the process creating another namespace will be allowed to
> > map only labels that it has permission to itself (those that it has in
> > its own map).
>
> > Special files inside the virtual smackfs needs to be reviewed whether
> > it's beneficial to have some of their functionality namespaced as well
> > (e.g. onlycap, syslog. ambient, etc). This would increase
> > CAP_MAC_ADMIN privileges inside the namespace.
> >
> >
> > Lukasz Pawelczyk (7):
> > user_ns: 3 new hooks for user namespace operations
> > smack: extend capability functions and fix 2 checks
> > smack: abstraction layer for 2 common Smack operations
> > smack: misc cleanups in preparation for a namespace patch
> > smack: namespace groundwork
> > smack: namespace implementation
> > smack: documentation for the Smack namespace
> >
> > Documentation/security/00-INDEX | 2 +
> > Documentation/security/Smack-namespace.txt | 231 +++++++++++++
> > MAINTAINERS | 1 +
> > fs/proc/base.c | 57 +++
> > include/linux/lsm_hooks.h | 28 ++
> > include/linux/security.h | 23 ++
> > include/linux/user_namespace.h | 9 +
> > kernel/user.c | 3 +
> > kernel/user_namespace.c | 18 +
> > security/security.c | 28 ++
> > security/smack/Kconfig | 12 +
> > security/smack/Makefile | 1 +
> > security/smack/smack.h | 187 +++++++++-
> > security/smack/smack_access.c | 191 ++++++++--
> > security/smack/smack_lsm.c | 536 ++++++++++++++++++++---------
> > security/smack/smack_ns.c | 471 +++++++++++++++++++++++++
> > security/smack/smackfs.c | 154 +++++----
> > 17 files changed, 1702 insertions(+), 250 deletions(-)
> > create mode 100644 Documentation/security/Smack-namespace.txt
> > create mode 100644 security/smack/smack_ns.c
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH 2/2] fbcon: use the cursor blink interval provided by vt
From: Andrey Wagin @ 2015-05-27 11:07 UTC (permalink / raw)
To: Scot Doyle
Cc: Greg Kroah-Hartman, Tomi Valkeinen,
Jean-Christophe Plagniol-Villard, Jiri Slaby, Michael Kerrisk,
Pavel Machek, Geert Uytterhoeven, LKML,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-man-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.11.1505270737270.6667@local>
2015-05-27 10:52 GMT+03:00 Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>:
> On Wed, 27 May 2015, Andrey Wagin wrote:
> ...
>> I regularly execute criu tests on linux-next. For this, I use virtual
>> machine from the digitalocean clould. The current version of
>> linux-next hangs after a few seconds. I use git bisect to find the
>> commit where the problem is appeaed. And it looks like the problem is
>> in this patch.
> ...
>> Thanks,
>> Andrew
>
> Perhaps this pending patch will help:
> http://marc.info/?l=linux-kernel&m=143219509918969&w=2
Yes, it helps. Thanks.
>
> Thanks,
> Scot
>
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Casey Schaufler @ 2015-05-27 13:05 UTC (permalink / raw)
To: Lukasz Pawelczyk
Cc: David S. Miller, Eric W. Biederman, Kirill A. Shutemov,
Serge E. Hallyn, Al Viro, Alexey Dobriyan, Andrew Morton,
Andy Lutomirski, David Howells, Fabian Frederick, Greg KH,
James Morris, Jeff Layton, Jingoo Han, Joe Perches, John Johansen,
Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab, Miklos Szeredi,
Oleg Nesterov, Paul Moore, Stephen Smalley
In-Reply-To: <1432718944.1784.1.camel@samsung.com>
On 5/27/2015 2:29 AM, Lukasz Pawelczyk wrote:
> On wto, 2015-05-26 at 18:04 -0700, Casey Schaufler wrote:
>> On 5/25/2015 5:32 AM, Lukasz Pawelczyk wrote:
>>> Hello,
>>>
>>> Some time ago I sent a Smack namespace documentation and a preliminary
>>> LSM namespace for RFC. I've been suggested that there shouldn't be a
>>> separate LSM namespace and that it should live within user namespace.
>>> And this version does. This is a complete set of patches required for
>>> Smack namespace.
>>>
>>> This was designed with a collaboration of Smack maintainer Casey
>>> Schaufler.
>>>
>> I have reviewed these patches (again!) and don't have
>> any issues. I see that Mr. Smalley has some insightful
>> suggestions, and once those are addressed I think that
>> we should be lined up to sell it to the infrastructure.
>
> Thank you.
>
> Can I add
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
>
> to those 7 the patches?
Yes, but please be sure to address the issues raised by Stephen and Eric.
^ permalink raw reply
* Re: [RFC v2 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-05-27 13:32 UTC (permalink / raw)
To: Greg KH
Cc: Jan Kara, linux-kernel, linux-fsdevel, linux-api, tytso,
adilger.kernel, hughd, lczerner, hch, linux-ext4, linux-mm,
kyungmin.park, kmpark
In-Reply-To: <20150527023412.GA20070@kroah.com>
On 05/27/2015 04:34 AM, Greg KH wrote:
> On Tue, May 26, 2015 at 06:39:48PM +0200, Beata Michalska wrote:
>> Hi,
>>
>> Things has gone a bit quiet thread wise ...
>> As I believe I've managed to snap back to reality, I was hoping we could continue with this?
>> I'm not sure if we've got everything cleared up or ... have we reached a dead end?
>> Please let me know if we can move to the next stage? Or, if there are any showstoppers?
>
> Please resend if you think it's ready and you have addressed the issues
> raised so far.
>
> thanks,
>
> greg k-h
>
Alright.
I'm still running some tests so I'll resend it most probably tomorrow
or on Friday.
Best Regards
Beata
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Stephen Smalley @ 2015-05-27 13:33 UTC (permalink / raw)
To: Lukasz Pawelczyk
Cc: David S. Miller, Eric W. Biederman, Kirill A. Shutemov,
Serge E. Hallyn, Al Viro, Alexey Dobriyan, Andrew Morton,
Andy Lutomirski, Casey Schaufler, David Howells, Fabian Frederick,
Greg KH, James Morris, Jeff Layton, Jingoo Han, Joe Perches,
John Johansen, Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab,
Miklos Szeredi, Oleg Nesterov, Paul Moore
In-Reply-To: <1432719372.1784.4.camel@samsung.com>
On 05/27/2015 05:36 AM, Lukasz Pawelczyk wrote:
> On wto, 2015-05-26 at 10:35 -0400, Stephen Smalley wrote:
>> On 05/25/2015 08:32 AM, Lukasz Pawelczyk wrote:
>>> --- Usage ---
>>>
>>> Smack namespace is written using LSM hooks inside user namespace. That
>>> means it's connected to it.
>>>
>>> To create a new Smack namespace you need to unshare() user namespace
>>> as usual. If that is all you do though, than there is no difference to
>>> what is now. To activate the Smack namespace you need to fill the
>>> labels' map. It is in a file /proc/$PID/smack_map.
>>
>> This should be /proc/$PID/attr/label_map or similar, modeled after the
>> existing /proc/$PID/attr/current and similar nodes. Then it isn't
>> module-specific and can be reused for other modules.
>
> To make this generic I'll have to introduce new LSH hooks to handle this
> file (much like /proc/$PID/attr/current).
> I take this is what you had in mind.
We don't have separate hooks for the
/proc/self/attr/{current,exec,fscreate,keycreate,socketcreate,prev}
nodes. Just one hook that passes the attribute name, and then the
security hook implementation can distinguish on that.
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Eric W. Biederman @ 2015-05-27 15:12 UTC (permalink / raw)
To: Lukasz Pawelczyk
Cc: Casey Schaufler, David S. Miller, Kirill A. Shutemov,
Serge E. Hallyn, Al Viro, Alexey Dobriyan, Andrew Morton,
Andy Lutomirski, David Howells, Fabian Frederick, Greg KH,
James Morris, Jeff Layton, Jingoo Han, Joe Perches, John Johansen,
Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab, Miklos Szeredi,
Oleg Nesterov, Paul Moore, Stephen Smalley
In-Reply-To: <1432721610.1784.27.camel@samsung.com>
Lukasz Pawelczyk <l.pawelczyk@samsung.com> writes:
> On wto, 2015-05-26 at 22:13 -0500, Eric W. Biederman wrote:
>> Lukasz Pawelczyk <l.pawelczyk@samsung.com> writes:
>>
>> > Hello,
>> >
>> > Some time ago I sent a Smack namespace documentation and a preliminary
>> > LSM namespace for RFC. I've been suggested that there shouldn't be a
>> > separate LSM namespace and that it should live within user namespace.
>> > And this version does. This is a complete set of patches required for
>> > Smack namespace.
>> >
>> > This was designed with a collaboration of Smack maintainer Casey
>> > Schaufler.
>> >
>> > Smack namespace have been implemented using user namespace hooks added
>> > by one of the patches. To put some context to it I paste here a
>> > documentation on what Smack namespace wants to achieve.
>> >
>> > LSM hooks themselves are documented in the security.h file inside the
>> > patch.
>> >
>> > The patches are based on:
>> > https://github.com/cschaufler/smack-next/tree/smack-for-4.2-stacked
>> >
>> > The tree with them is avaiable here:
>> > https://github.com/Havner/smack-namespace/tree/smack-namespace-for-4.2-stacked-v2
>> >
>> > Changes from v1:
>> > - "kernel/exit.c: make sure current's nsproxy != NULL while checking
>> > caps" patch has been dropped
>> > - fixed the title of the user_ns operations patch
>>
>> A have not completed a review I don't understand smack well enough to
>> answer some questions but I don't think I like the approach this patch
>> takes to get things done.
>>
>> I am flattered that you are using a mapping as I did with uid map and
>> gid map. Unfortunately I do not believe your mapping achieves what my
>> mapping of uids and gids achieved.
>>
>> A technical goal is to give people the tools so that a sysadmin can set
>> up a system, can grant people subids and subgids, and then the user can
>> proceed to do what they need to do.
>
> This is exactly the case with Smack namespace.
>
>
>> In particular there should be
>> little to no need to keep pestering the system administrator for more
>> identifiers.
>
> This all depends on the use case. When you create a new namespace for a
> particular purpose you could know what labels will be required there and
> map them upfront. Adding new mappings is for a use case that a container
> might have new software installed that requires new labels for it. You
> don't create/import new labels on a normal basis all the time.
Good.
>> The flip side of that was that the mapping would ensure all of the
>> existing permissions checks would work as expected, and the checks in
>> the kernel could be converted without much trouble.
>
> Again, this is exactly the case with Smack namespace. There are
> additional checks in place of course, but the rules are clear about
> how that works.
>
>
>> Ranges of ids were choosen because they allow for a lot of possible ways
>> of using uids and gids in containers, are comparitively easy to
>> administer, are very fast to use, and don't need large data structures.
>
> This is Smack specific. The same way we could compare Smack with DAC
> (even without talking about namespaces). Every new UID is just it, a
> number. Every new label requires allocation and adds to the list.
>
> I had an idea about mapping ranges of labels for a container by using
> prefixes but this was dropped by Casey. Labels (with an exception of the
> built-in ones) should not have any special meaning. This is Smack
> design.
Maybe. All I care about is the resources for user namespaces remain
something that the administrator can set up (before any containers
exist) a set of resources (uid, gids, labels?) that an individual user
can use and then that user can set up containers as they desire.
That is the property that the administration of the container is fully
delegated to the whoever creates the container.
> About performance, as it was originally with Smack. The list of labels
> was a simple list. When the performance was not enough hashing was
> added. I'd take the same approach here. If some use cases will require
> so many mappings that this becomes a problem we will deal with it. I
> don't want to complicate the patches now and I this was Casey's opinion
> as well.
Occassionally performance implications are sufficiently profound that
it impacts your userspace API, and thus all future maintenance. How
labels are managed and mapped into a container appears to be one such
issue. So unfortunately it does not appear to shove performance
questions off until later.
And actually it is much more about managability rather than raw
performance.
What I know is that the human factors of how these identifiers are
assigned and managed is important. Especially as they are persistent on
disk and likely need to be kept consistent between multiple different
machines.
A range of uids, a prefix on a label, those kinds of things are simple
and easy to understand and make sense of. The rule is simple enough I
can track it in my head and I don't need to keep going back and looking
at configuration files. Discrete unrelated assignment of values
(unless the set of values is very small) does not work that way and
makes things less managable.
Which leads to my observation that if the mapping rule is simple enough
I can keep track of it in my head I can put a small array in struct
user_namespace to implement that rule.
>> With a discreet mapping of labels I have the premonition that we now
>> have a large data structure that, is not as flexible in to use,
>> is comparatively slow and appears to require an interaction with the
>> system administrator for every label you use in a container.
>
> Again, DAC vs Smack philosophy. The same way a new label appears in init
> namespace (e.g. with a file/inode). Without a new rule you won't have an
> access to it. This requires administrator intervention as well. This is
> MAC after all.
If it does not work with how user namespaces are designed to be used
this approach gets my nack. I will not accept an approach that requires
asking a system administrator permission for every little change. That
fundamentally is what the user namespace gets away from.
If on the other hand the sysadmin interaction becomes here are N labels
do what you need to with them. Where N is sufficiently large that most
users can be given those N labels and then users don't have to ask about
it I can live with that.
>> As part of that there is added a void *security pointer in the user
>> namespace to apparently hang off anything anyone would like to use.
>> Connected to that are hooks that have failure codes (presumably memory
>> allocation failures), but the semantics are not clear. My gut feel is
>> that I would rather extend struct user_namespace to hold the smack label
>> mapping table and remove all of the error codes because they would then
>> be unnecessary.
>
> How is this different then filling a void *security pointer for other
> kernel objects (tasks, indodes, ipc, etc). The allocations can fail as
> well. I don't see how the semantics are different here.
>
> So you would have me a Smack generic pointer in user_namespace without
> any LSM abstraction? How does that cope with LSM philosophy and recently
> introduced (initial) LSM stacking?
I would have an smack specific array of mapped labels (probably in a
union).
I am not an LSM guy and to me tell the only important LSM philosophy is
provide a way for the various people who want to odd things with
security to each have a place at the table so they don't fight.
What you are proposing is something different from core LSM activities.
I have a hard time working with ``security'' code in the kernel as
almost invariably it is some of the worst code in the core kernel.
There are likely a lot of reasons for that but one reason that stands
out to me is the LSM interface seems to relegate the ``security'' code
to second class status.
We have merged the security modules now and we no longer support
loadable modules so I think some of the original design assumptions can
be reexamined. If we will change anything upon reexamination I don't
know and I am not looking for any great whole sale change. But I do
intend to look at LSM related changes to the code I maintain under the
same spot light and under the same criteria as I look at any other
changes.
i.e. I can't ignore what the LSMs do during code maintenance so I won't
ignore what the LSMs do or want to do when adding LSM hooks.
>> I am also concerned that several of the operations assume that setns
>> and the like are normally privileged operations and so require the
>> ability to perform other privileged operations. Given that in the right
>> circumstances setns is not privileged that seems like a semantics
>> mismatch.
>
> Sorry, I don't exactly know what you mean. setns remains unprivileged
> operation here. The only limit is to refuse setns when adding a process
> to a container would break Smack namespace rules. In theory this
> limitation could be removed (I think), because a process with an
> unmapped label would not be able to do anything in the namespace. This
> is mostly for convenience.
The rule may make sense but because of it it sounds like in practice you
may actually have to be root to enter a container.
>> Or in short my gut feel says the semantics of this change are close to
>> something that would be very useful, but the details make this patchset
>> far less useful, usable and comprehensible than it should be.
>
> This isn't much to go on for me unfortunately.
> Anyway, thank you for your insight, and taking the time.
Eric
^ permalink raw reply
* Re: [klibc] kernel/libc uapi changes for y2038
From: H. Peter Anvin @ 2015-05-27 15:28 UTC (permalink / raw)
To: Arnd Bergmann, linux-api
Cc: klibc, libc-alpha, y2038, musl, linux-kernel, Rich Felker,
cferris, enh, Joseph S. Myers
In-Reply-To: <2664016.bYZKg6FQqR@wuerfel>
On 05/18/2015 02:53 AM, Arnd Bergmann wrote:
> In the patch series I posted recently [1], I introduce new system calls to deal
> with modified data structures, but left the question open on how these should
> be best accessed from libc. The patches introduce a new __kernel_time64_t type
> and based on that five new data structured: struct __kernel_timespec,
> struct __kernel_itimerspec, struct __kernel_stat, struct __kernel_rusage,
> and struct __kernel_timex. This works fine for the case when all libc
> implementations provide their own definitions to user space, but not for
> the simplest case (e.g. klibc) where the user-visible structures come directly
> from the kernel uapi headers.
>
> I still don't know what model the various libc developers prefer, so here is
> an alternative approach, as a patch on top of the previous series:
>
> Now, we rename the original structures to struct __old_kernel_*, and use a
> macro to define either the __old_kernel_* or the __kernel_* structure name
> to the name we actually want in user space, based on a __KERNEL_TIME_BITS
> macro that can be set to either 32 or 64 for 32-bit architectures by
> the libc. Depending on that macro, the compiler will either see one
> of these combinations (for each of the five structures):
>
> a) __BITS_PER_LONG == 32 && __KERNEL_TIME_BITS == 32:
>
> struct timespec based on 32-bit __kernel_time_t
> struct __kernel_timespec based on 64-bit __kernel_time64_t
>
> b) __BITS_PER_LONG == 64 && __KERNEL_TIME_BITS == 64:
>
> struct timespec based on 64-bit __kernel_time_t
> struct __kernel_timespec based on 64-bit __kernel_time64_t
>
> c) __BITS_PER_LONG == 32 && __KERNEL_TIME_BITS == 64:
>
> struct __old_kernel_timespec based on 32-bit __kernel_time_t
> struct timespec based on 64-bit __kernel_time64_t
>
> Would this work for everyone? Any alternative suggestions?
>
It seems to work, except I don't really understand why there is a
difference between (b) and (c).
I also have no problem just having klibc contain its own definitions of
these structures, as long as one can prevent the kernel from defining them.
-hpa
^ permalink raw reply
* Re: [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU
From: Peter Zijlstra @ 2015-05-27 15:39 UTC (permalink / raw)
To: Robert Bragg
Cc: dri-devel, David Airlie, linux-api, intel-gfx, linux-kernel,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Daniel Vetter
In-Reply-To: <CAMou1-2Wgs9nvw9w8CHoacix8YSVAmuKu4MZJC=QQuCbLAx72w@mail.gmail.com>
On Thu, May 21, 2015 at 12:17:48AM +0100, Robert Bragg wrote:
> >
> > So for me the 'natural' way to represent this in perf would be through
> > event groups. Create a perf event for every single event -- yes this is
> > 53 events.
>
> So when I was first looking at this work I had considered the
> possibility of separate events, and these are some of the things that
> in the end made me forward the hardware's raw report data via a single
> event instead...
>
> There are 100s of possible B counters depending on the MUX
> configuration + fixed-function logic in addition to the A counters.
There's only 8 B counters, what there is 100s of is configurations, and
that's no different from any other PMU. There's 100s of events to select
on the CPU side as well - yet only 4 (general purpose) counters (8 if
you disable HT on recent machines) per CPU.
Of course, you could create more than 8 events at any one time, and then
perf will round-robin the configuration for you.
> A
> choice would need to be made about whether to expose events for the
> configurable counters that aren't inherently associated with any
> semantics, or instead defining events for counters with specific
> semantics (with 100s of possible counters to define). The later would
> seem more complex for userspace and the kernel if they both now have
> to understand the constraints on what counters can be used together.
Again, no different from existing PMUs. The most 'interesting' part of
any PMU driver is event scheduling.
The explicit design of perf was to put event scheduling _in_ the kernel
and not allow userspace direct access to the hardware (as previous PMU
models did).
> I
> guess with either approach we would also need to have some form of
> dedicated group leader event accepting attributes for configuring the
> state that affects the group as a whole, such as the counter
> configuration (3D vs GPGPU vs media etc).
That depends a bit on how flexible you want to switch between these
modes; with the cost being in the 100s of MMIO register writes, it might
just not make sense to make this too dynamic.
An option might be to select the mode through the PMU driver's sysfs
files. Another option might be to expose the B counters 3 times and have
each set be mutually exclusive. That is, as soon as you've created a 3D
event, you can no longer create GPGPU/Media events.
> I'm not sure where we would
> handle the context-id + drm file descriptor attributes for initiating
> single context profiling but guess we'd need to authenticate each
> individual event open.
Event open is a slow path, so that should not be a problem, right?
> It's not clear if we'd configure the report
> layout via the group leader, or try to automatically choose the most
> compact format based on the group members. I'm not sure how pmus
> currently handle the opening of enabled events on an enabled group but
With the PERF_FORMAT_GROUP layout changing in-flight. I would recommend
not doing that -- decoding the output will be 'interesting' but not
impossible.
> I think there would need to be limitations in our case that new
> members can't result in a reconfigure of the counters if that might
> loose the current counter values known to userspace.
I'm not entirely sure what you mean here.
> From a user's pov, there's no real freedom to mix and match which
> counters are configured together, and there's only some limited
> ability to ignore some of the currently selected counters by not
> including them in reports.
It is 'impossible' to create a group that is not programmable. That is,
the pmu::event_init() call _should_ verify that the addition of the
event to the group (as given by event->group_leader) is valid.
> Something to understand here is that we have to work with sets of
> pre-defined MUX + fixed-function logic configurations that have been
> validated to give useful metrics for specific use cases, such as
> benchmarking 3D rendering, GPGPU or media workloads.
This is fine; as stated above. Since these are limited pieces of
'firmware' which are expensive to load, you don't have to do a fully
dynamic solution here.
> As it is currently the kernel doesn't need to know anything about the
> semantics of individual counters being selected, so it's currently
> convenient that we can aim to maintain all the counter meta data we
> have in userspace according to the changing needs of tools or drivers
> (e.g. names, descriptions, units, max values, normalization
> equations), de-coupled from the kernel, instead of splitting it
> between the kernel and userspace.
And that fully violates the design premise of perf. The kernel should be
in control of resources, not userspace.
If we'd have put userspace in charge, we could now not profile the same
task by two (or more) different observers. But with kernel side counter
management that is no problem at all.
> A benefit of being able to change the report size is to reduce memory
> bandwidth usage that can skew measurements. It's possible to request
> the gpu to write out periodic snapshots at a very high frequency (we
> can program a period as low as 160 nanoseconds) and higher frequencies
> can start to expose some interesting details about how the gpu is
> utilized - though with notable observer effects too. How careful we
> are to not waste bandwidth is expected to determine what sampling
> resolutions we can achieve before significantly impacting what we are
> measuring.
>
> Splitting the counters up looked like it could increase the bandwidth
> we use quite a bit. The main difference comes from requiring 64bit
> values instead of the 32bit values in our raw reports. This can be
> offset partly since there are quite a few 'reserved'/redundant A
> counters that don't need forwarding. As an example in the most extreme
> case, instead of an 8 byte perf_event_header + 4byte raw_size + 256
> byte reports + 4 byte padding every 160ns ~= 1.5GB/s, we might have 33
> A counters (ignoring redundant ones) + 16 configurable counters = 400
In that PDF there's only 8 configurable 'B' counters.
> byte struct read_format (using PERF_FORMAT_GROUP) + 8 byte
> perf_event_header every 160ns ~= 2.4GB/s. On the other hand though we
> could choose to forward only 2 or 3 counters of interest at these high
> frequencies which isn't possible currently.
Right; although you'd have to spend some cpu cycles on the format shift.
Which might make it moot again. That said; I would really prefer we
start out with trying to make the generic format stuff work before
trying to come up with special case hacks.
> > Use the MMIO reads for the regular read() interface, and use a hrtimer
> > placing MI_REPORT_PERF_COUNT commands, with a counter select mask
> > covering the all events in the current group, for sampling.
>
> Unfortunately due to the mmio limitations and the need to relate
> counters I can't imagine many use cases for directly accessing the
> counters individually via the read() interface.
Fair enough.
> MI_REPORT_PERF_COUNT commands are really only intended for collecting
> reports in sync with a command stream. We are experimenting currently
> with an extension of my PMU driver that emits MI_REPORT_PERF_COUNT
> commands automatically around the batches of commands submitted by
> userspace so we can do a better job of filtering metrics across many
> gpu contexts, but for now the expectation is that the kernel shouldn't
> be emitting MI_REPORT_PERF_COUNT commands. We emit
> MI_REPORT_PERF_COUNT commands within Mesa for example to implement the
> GL_INTEL_performance_query extension, at the start and end of a query
> around a sequence of commands that the application is interested in
> measuring.
I will try and read up on the GL_INTEL_performance_query thing.
> > You can use the perf_event_attr::config to select the counter (A0-A44,
> > B0-B7) and use perf_event_attr::config1 (low and high dword) for the
> > corresponding CEC registers.
> >
>
> Hopefully covered above, but since the fixed-function state is so
> dependent on the MUX configuration I think it currently makes sense to
> treat the MUX plus logic state (including the CEC state) a tightly
> coupled unit.
Oh wait, the A counters are also affected by the MUX programming!
That wasn't clear to me before this.
Same difference though; you could still do either the sysfs aided flips
or the mutually exclusive counter types to deal with this; all still
assuming reprogramming all that is 'expensive'.
> The Flexible EU Counters for Broadwell+ could be more amenable to this
> kind of independent configuration, as I don't believe they are
> dependant on the MUX configuration.
That sounds good :-)
> One idea that's come up a lot though is having the possibility of
> being able to configure an event with a full MUX + fixed-function
> state description.
Right; seeing how there's only a very limited number of these MUX
programs validated (3D, GPGPU, Media, any others?) this should be
doable. And as mentioned before, you could disable the other types once
you create one in order to limit the reprogramming thing.
> >
> > This does not require random per driver ABI extentions for
> > perf_event_attr, nor your custom output format.
> >
> > Am I missing something obvious here?
>
> Definitely nothing 'obvious' since the current documentation is
> notably incomplete a.t.m, but I don't think we were on the same page
> about how the hardware works and our use cases.
>
> Hopefully some of my above comments help clarify some details.
Yes, thanks!
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Tomas Winkler @ 2015-05-27 15:42 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: arnd-r2nGTMty4D4, Stephen Rothwell, Tomas Winkler,
linux-api-u79uwXL29TY76Z2rM5mHXA
In order for mei client devices to use device id based on uuid we
have to use common types between user space (file2alias.c).
Similarly to vmbus, mei uses raw 16 byte array for that.
To leverage on existing infrastructure around uuid_le type
defined in uuid.h we add helper macros to handle conversions between
raw 16 byte array and uuid_{le,be} types.
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
V2: be verbose in the commit message
include/uapi/linux/uuid.h | 41 ++++++++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 17 deletions(-)
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 786f0773cc33..487f098c8517 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -32,27 +32,34 @@ typedef struct {
__u8 b[16];
} uuid_be;
-#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_le) \
-{{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
- (b) & 0xff, ((b) >> 8) & 0xff, \
- (c) & 0xff, ((c) >> 8) & 0xff, \
- (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+#define __UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+ {(a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff,\
+ (b) & 0xff, ((b) >> 8) & 0xff, \
+ (c) & 0xff, ((c) >> 8) & 0xff, \
+ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7)}
+
+#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+ ((uuid_le) {__UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)})
+
+#define __UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+ {((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff,\
+ ((b) >> 8) & 0xff, (b) & 0xff, \
+ ((c) >> 8) & 0xff, (c) & 0xff, \
+ (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7)}
#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
-((uuid_be) \
-{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
- ((b) >> 8) & 0xff, (b) & 0xff, \
- ((c) >> 8) & 0xff, (c) & 0xff, \
- (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+ ((uuid_be) {__UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)})
+
+#define __NULL_UUID_LE \
+ __UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
+ 0x00, 0x00, 0x00, 0x00)
-#define NULL_UUID_LE \
- UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
- 0x00, 0x00, 0x00, 0x00)
+#define NULL_UUID_LE ((uuid_le) {__NULL_UUID_LE})
-#define NULL_UUID_BE \
- UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
- 0x00, 0x00, 0x00, 0x00)
+#define __NULL_UUID_BE \
+ __UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00, \
+ 0x00, 0x00, 0x00, 0x00)
+#define NULL_UUID_BE ((uuid_be) {__NULL_UUID_BE})
#endif /* _UAPI_LINUX_UUID_H_ */
--
2.1.0
^ permalink raw reply related
* Re: [RFC PATCH 00/11] drm/i915: Expose OA metrics via perf PMU
From: Ingo Molnar @ 2015-05-27 16:41 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Robert Bragg, intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
Daniel Vetter, Jani Nikula, David Airlie, Paul Mackerras,
Ingo Molnar, Arnaldo Carvalho de Melo,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150527153914.GC3644-ndre7Fmf5hadTX5a5knrm8zTDFooKrT+cvkQGrU6aU0@public.gmane.org>
* Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> > As it is currently the kernel doesn't need to know anything about the
> > semantics of individual counters being selected, so it's currently convenient
> > that we can aim to maintain all the counter meta data we have in userspace
> > according to the changing needs of tools or drivers (e.g. names, descriptions,
> > units, max values, normalization equations), de-coupled from the kernel,
> > instead of splitting it between the kernel and userspace.
>
> And that fully violates the design premise of perf. The kernel should be in
> control of resources, not userspace.
>
> If we'd have put userspace in charge, we could now not profile the same task by
> two (or more) different observers. But with kernel side counter management that
> is no problem at all.
Beyond that there are a couple of other advantages of managing performance metrics
via the kernel:
2)
Per task/workload/user separation of events, for example you can do:
perf record make bzImage
perf stat make bzImage
These will only profile/count in the child task hierarchy spawned by the shell.
Multiple such sessions can be started and the perf sampling/counting of the
separate workloads is independent of each other.
3)
Finegrained security model: you can chose which events to expose to what privilege
levels. You can also work around hardware bugs without complicating tooling.
4)
Seemless integration of the various PMU and other event metrics with each other:
you can start a CPU PMU event, a trace event, a software event and an uncore
event, which all output into the same coherent, ordered ring-buffer, timestamped
and correlated consistently, etc.
5)
The unified event model on the kernel side also brings with itself 100 KLOC of
rich, unified tooling in tools/perf/, which tries to keep it all together and
functional.
================
The cost of all that is having to implement kernel support for it, but once that
initial hurdle is taken, it's well worth it! A PMU driver can be as simple as
offering only a single system-wide counter.
Thanks,
Ingo
^ permalink raw reply
* Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Greg KH @ 2015-05-27 16:50 UTC (permalink / raw)
To: Tomas Winkler
Cc: arnd-r2nGTMty4D4, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1432741333-11889-1-git-send-email-tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> In order for mei client devices to use device id based on uuid we
> have to use common types between user space (file2alias.c).
> Similarly to vmbus, mei uses raw 16 byte array for that.
> To leverage on existing infrastructure around uuid_le type
> defined in uuid.h we add helper macros to handle conversions between
> raw 16 byte array and uuid_{le,be} types.
You aren't adding a helper macro, you are just redefining the existing
macros using a different one. But I can't see why this is needed, what
does this solve that vmbus and other uses of the existing macros don't
need? In other words, what makes mei so special that it needs a "lower"
level macro than every other subsystem?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 0/7] Smack namespace
From: Lukasz Pawelczyk @ 2015-05-27 17:15 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Casey Schaufler, David S. Miller, Kirill A. Shutemov,
Serge E. Hallyn, Al Viro, Alexey Dobriyan, Andrew Morton,
Andy Lutomirski, David Howells, Fabian Frederick, Greg KH,
James Morris, Jeff Layton, Jingoo Han, Joe Perches, John Johansen,
Jonathan Corbet, Kees Cook, Mauro Carvalho Chehab, Miklos Szeredi,
Oleg Nesterov, Paul Moore, Stephen Smalley
In-Reply-To: <87vbfeqcke.fsf@x220.int.ebiederm.org>
On śro, 2015-05-27 at 10:12 -0500, Eric W. Biederman wrote:
> Lukasz Pawelczyk <l.pawelczyk@samsung.com> writes:
> > On wto, 2015-05-26 at 22:13 -0500, Eric W. Biederman wrote:
> >> In particular there should be
> >> little to no need to keep pestering the system administrator for more
> >> identifiers.
> >
> > This all depends on the use case. When you create a new namespace for a
> > particular purpose you could know what labels will be required there and
> > map them upfront. Adding new mappings is for a use case that a container
> > might have new software installed that requires new labels for it. You
> > don't create/import new labels on a normal basis all the time.
>
> Good.
>
> >> The flip side of that was that the mapping would ensure all of the
> >> existing permissions checks would work as expected, and the checks in
> >> the kernel could be converted without much trouble.
> >
> > Again, this is exactly the case with Smack namespace. There are
> > additional checks in place of course, but the rules are clear about
> > how that works.
> >
> >
> >> Ranges of ids were choosen because they allow for a lot of possible ways
> >> of using uids and gids in containers, are comparitively easy to
> >> administer, are very fast to use, and don't need large data structures.
> >
> > This is Smack specific. The same way we could compare Smack with DAC
> > (even without talking about namespaces). Every new UID is just it, a
> > number. Every new label requires allocation and adds to the list.
> >
> > I had an idea about mapping ranges of labels for a container by using
> > prefixes but this was dropped by Casey. Labels (with an exception of the
> > built-in ones) should not have any special meaning. This is Smack
> > design.
>
> Maybe. All I care about is the resources for user namespaces remain
> something that the administrator can set up (before any containers
> exist) a set of resources (uid, gids, labels?) that an individual user
> can use and then that user can set up containers as they desire.
>
> That is the property that the administration of the container is fully
> delegated to the whoever creates the container.
You can do that the same way as in user namespace.
You are allowed to add mappings later, but e.g. if you have a hermetic
container you don't have to. This is not required for a Smack namespace
to work. It's purely optional. You can map a set of labels and forget
about it. The namespace will not be able to import/create a new ones but
it will work just fine.
> > About performance, as it was originally with Smack. The list of labels
> > was a simple list. When the performance was not enough hashing was
> > added. I'd take the same approach here. If some use cases will require
> > so many mappings that this becomes a problem we will deal with it. I
> > don't want to complicate the patches now and I this was Casey's opinion
> > as well.
>
> Occassionally performance implications are sufficiently profound that
> it impacts your userspace API, and thus all future maintenance. How
> labels are managed and mapped into a container appears to be one such
> issue. So unfortunately it does not appear to shove performance
> questions off until later.
In this case the only "public" kernel API are user_ns hooks that are
orthogonal to the Smack internals, so I don't really see how is that a
case.
I was also asked by Stephen Smalley to make the label map a generic file
in /proc/$PID/attr/ so it will also be up to the LSM to decide how this
file is treated. It will be abstracted by getprocattr/setprocattr hooks
or something similar.
> And actually it is much more about managability rather than raw
> performance.
>
> What I know is that the human factors of how these identifiers are
> assigned and managed is important. Especially as they are persistent on
> disk and likely need to be kept consistent between multiple different
> machines.
>
> A range of uids, a prefix on a label, those kinds of things are simple
> and easy to understand and make sense of. The rule is simple enough I
> can track it in my head and I don't need to keep going back and looking
> at configuration files. Discrete unrelated assignment of values
> (unless the set of values is very small) does not work that way and
> makes things less managable.
Like I said, this is Smack design (fortunately or not). I don't really
see how that could be different here. We might argue why Smack uses
strings instead of numbers. But by design those strings should bear no
special meaning and Casey stressed this on several occasions.
Nobody prevents you from doing prefixes yourself though:
C1_label1 -> label1
C1_label2 -> label2
etc..
> Which leads to my observation that if the mapping rule is simple enough
> I can keep track of it in my head I can put a small array in struct
> user_namespace to implement that rule.
Even though you can only have 5 mappings in user_ns you could in
principle map the UIDs in a way that would be confusing. The 5 mappings
mitigate that significantly of course, but numbers are easier to deal
with than strings. It's just the way it is.
> >> With a discreet mapping of labels I have the premonition that we now
> >> have a large data structure that, is not as flexible in to use,
> >> is comparatively slow and appears to require an interaction with the
> >> system administrator for every label you use in a container.
> >
> > Again, DAC vs Smack philosophy. The same way a new label appears in init
> > namespace (e.g. with a file/inode). Without a new rule you won't have an
> > access to it. This requires administrator intervention as well. This is
> > MAC after all.
>
> If it does not work with how user namespaces are designed to be used
> this approach gets my nack. I will not accept an approach that requires
> asking a system administrator permission for every little change. That
> fundamentally is what the user namespace gets away from.
>
> If on the other hand the sysadmin interaction becomes here are N labels
> do what you need to with them. Where N is sufficiently large that most
> users can be given those N labels and then users don't have to ask about
> it I can live with that.
This works here as I described earlier in this email.
> >> As part of that there is added a void *security pointer in the user
> >> namespace to apparently hang off anything anyone would like to use.
> >> Connected to that are hooks that have failure codes (presumably memory
> >> allocation failures), but the semantics are not clear. My gut feel is
> >> that I would rather extend struct user_namespace to hold the smack label
> >> mapping table and remove all of the error codes because they would then
> >> be unnecessary.
> >
> > How is this different then filling a void *security pointer for other
> > kernel objects (tasks, indodes, ipc, etc). The allocations can fail as
> > well. I don't see how the semantics are different here.
> >
> > So you would have me a Smack generic pointer in user_namespace without
> > any LSM abstraction? How does that cope with LSM philosophy and recently
> > introduced (initial) LSM stacking?
>
> I would have an smack specific array of mapped labels (probably in a
> union).
>
> I am not an LSM guy and to me tell the only important LSM philosophy is
> provide a way for the various people who want to odd things with
> security to each have a place at the table so they don't fight.
>
> What you are proposing is something different from core LSM activities.
I think this actually is very much in line with LSM activities. Every
object with a security context gets an opaque pointer that LSM
interprets as it wants. And does whatever it wants with it to make
security decisions.
> I have a hard time working with ``security'' code in the kernel as
> almost invariably it is some of the worst code in the core kernel.
> There are likely a lot of reasons for that but one reason that stands
> out to me is the LSM interface seems to relegate the ``security'' code
> to second class status.
>
> We have merged the security modules now and we no longer support
> loadable modules so I think some of the original design assumptions can
> be reexamined.
This is new to me and recently I have heard quite the opposite in regard
to the newly introduced LSM stacking code. I'm not aware that
out-of-tree modules should not be supported.
People do have and use out-of-tree LSM modules. Not having an opaque
pointer blocks them from using those LSM hooks.
And recent stacking changes try to make this infrastructure even more
generic, by proving stacked security pointers (hooks should already be
stacked in 4.2 afaik):
http://www.spinics.net/linux/fedora/linux-security-module/msg21608.html
> If we will change anything upon reexamination I don't
> know and I am not looking for any great whole sale change. But I do
> intend to look at LSM related changes to the code I maintain under the
> same spot light and under the same criteria as I look at any other
> changes.
>
> i.e. I can't ignore what the LSMs do during code maintenance so I won't
> ignore what the LSMs do or want to do when adding LSM hooks.
>
> >> I am also concerned that several of the operations assume that setns
> >> and the like are normally privileged operations and so require the
> >> ability to perform other privileged operations. Given that in the right
> >> circumstances setns is not privileged that seems like a semantics
> >> mismatch.
> >
> > Sorry, I don't exactly know what you mean. setns remains unprivileged
> > operation here. The only limit is to refuse setns when adding a process
> > to a container would break Smack namespace rules. In theory this
> > limitation could be removed (I think), because a process with an
> > unmapped label would not be able to do anything in the namespace. This
> > is mostly for convenience.
>
> The rule may make sense but because of it it sounds like in practice you
> may actually have to be root to enter a container.
In this particular case UID is irrelevant. You'll either be allowed or
not. Depending on Smack rules.
If the check would be removed UID would still be irrelevant. You'd
always be allowed (as far as Smack is concerned of course) but depending
on Smack rules the process would be usable or not (not able to access
anything, even itself).
I think you can't try to compare apples and oranges. Smack security
model, or any MAC for that matter will not be analogous to DAC. Being
root can be completely irrelevant for an LSM module. And it's completely
its own decision.
If a hook is added for some syscall a module might have its arbitrary
rules for allowing it or not. I don't see anything awkward here. And
honestly I think that having a hook for setns where it might be
allowed/refused could be beneficial to other LSM modules.
The hook itself doesn't define any decision/model. A module does.
As a side note: I do realize that having security context in user
namespace makes it kinda more of a security namespace (DAC, MAC, CAPS,
etc).
Initially I made a whole separate namespace for LSM. You told me this
was a strange decision and I agree now.
This extends user namespace a little and I don't feel this is bad. Even
now user_ns is not a part of nsproxy, but cred. It's created first and
referenced from all the other namespaces. It doesn't only map UIDs. It
also gives capabilities a context. So in the same way it could hold MAC
security context for MAC related decisions that don't necessarily align
with DAC (that's what MAC is for after all).
--
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* RE: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Winkler, Tomas @ 2015-05-27 17:24 UTC (permalink / raw)
To: Greg KH
Cc: arnd-r2nGTMty4D4@public.gmane.org, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150527165054.GB3533-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
>
> On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > In order for mei client devices to use device id based on uuid we
> > have to use common types between user space (file2alias.c).
> > Similarly to vmbus, mei uses raw 16 byte array for that.
> > To leverage on existing infrastructure around uuid_le type
> > defined in uuid.h we add helper macros to handle conversions between
> > raw 16 byte array and uuid_{le,be} types.
>
> You aren't adding a helper macro, you are just redefining the existing
> macros using a different one.
Not exactly I'm using both the one I've added for device ids and the old one for all the other flows.
But I can't see why this is needed, what
> does this solve that vmbus and other uses of the existing macros don't
> need? In other words, what makes mei so special that it needs a "lower"
> level macro than every other subsystem?
It's not special there is actually a lot of code duplication around uuid handling
every subsystem is using their own macros but it can be consolidated around uuid.h
So vmbus can use that
Instead of
/*
* Network GUID
* {f8615163-df3e-46c5-913f-f2d2f965ed0e}
*/
#define HV_NIC_GUID \
.guid = { \
0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
}
The can use the new macro to make it more readable, something in spirit of:
#define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-f2d2f965ed0e)
Thanks
Tomas
^ permalink raw reply
* Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Greg KH @ 2015-05-27 17:29 UTC (permalink / raw)
To: Winkler, Tomas
Cc: arnd-r2nGTMty4D4@public.gmane.org, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B3C4A2C8C-Jy8z56yoSI/jxeytcECX8bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:
>
> >
> > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > > In order for mei client devices to use device id based on uuid we
> > > have to use common types between user space (file2alias.c).
> > > Similarly to vmbus, mei uses raw 16 byte array for that.
> > > To leverage on existing infrastructure around uuid_le type
> > > defined in uuid.h we add helper macros to handle conversions between
> > > raw 16 byte array and uuid_{le,be} types.
> >
> > You aren't adding a helper macro, you are just redefining the existing
> > macros using a different one.
>
> Not exactly I'm using both the one I've added for device ids and the old one for all the other flows.
>
> But I can't see why this is needed, what
> > does this solve that vmbus and other uses of the existing macros don't
> > need? In other words, what makes mei so special that it needs a "lower"
> > level macro than every other subsystem?
>
> It's not special there is actually a lot of code duplication around uuid handling
> every subsystem is using their own macros but it can be consolidated around uuid.h
>
> So vmbus can use that
> Instead of
> /*
> * Network GUID
> * {f8615163-df3e-46c5-913f-f2d2f965ed0e}
> */
> #define HV_NIC_GUID \
> .guid = { \
> 0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
> 0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
> }
>
> The can use the new macro to make it more readable, something in spirit of:
>
> #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-f2d2f965ed0e)
Why the "__" usage here? That signifies a "private" namespace, why add
that to the user visible header files?
And are you going to send patches for vmbus and other drivers to fix
everything up to use these new macros? Someone has to...
thanks,
greg k-h
^ permalink raw reply
* RE: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Winkler, Tomas @ 2015-05-27 17:42 UTC (permalink / raw)
To: Greg KH
Cc: arnd-r2nGTMty4D4@public.gmane.org, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150527172919.GA5525-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
> -----Original Message-----
> From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> Sent: Wednesday, May 27, 2015 20:29
> To: Winkler, Tomas
> Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw
> arrays
>
> On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:
> >
> > >
> > > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > > > In order for mei client devices to use device id based on uuid we
> > > > have to use common types between user space (file2alias.c).
> > > > Similarly to vmbus, mei uses raw 16 byte array for that.
> > > > To leverage on existing infrastructure around uuid_le type
> > > > defined in uuid.h we add helper macros to handle conversions between
> > > > raw 16 byte array and uuid_{le,be} types.
> > >
> > > You aren't adding a helper macro, you are just redefining the existing
> > > macros using a different one.
> >
> > Not exactly I'm using both the one I've added for device ids and the old one for
> all the other flows.
> >
> > But I can't see why this is needed, what
> > > does this solve that vmbus and other uses of the existing macros don't
> > > need? In other words, what makes mei so special that it needs a "lower"
> > > level macro than every other subsystem?
> >
> > It's not special there is actually a lot of code duplication around uuid handling
> > every subsystem is using their own macros but it can be consolidated around
> uuid.h
> >
> > So vmbus can use that
> > Instead of
> > /*
> > * Network GUID
> > * {f8615163-df3e-46c5-913f-f2d2f965ed0e}
> > */
> > #define HV_NIC_GUID \
> > .guid = { \
> > 0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
> > 0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
> > }
> >
> > The can use the new macro to make it more readable, something in spirit of:
> >
> > #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-f2d2f965ed0e)
>
> Why the "__" usage here? That signifies a "private" namespace, why add
> that to the user visible header files?
I take any other suggestion for macro names.
Not sure how I would reuse the macros if I don't export them both, second this can be used also by user space.
>
> And are you going to send patches for vmbus and other drivers to fix
> everything up to use these new macros? Someone has to...
Can be done but I cannot test their code and now I'm busy with splitting the big bus patch :)
Tomas
^ permalink raw reply
* Re: [klibc] kernel/libc uapi changes for y2038
From: Arnd Bergmann @ 2015-05-27 20:19 UTC (permalink / raw)
To: H. Peter Anvin
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, klibc-YMNOUZJC4hwAvxtiuMwx3w,
libc-alpha-9JcytcrH/bA+uJoB2kUjGw, y2038-cunTk1MwBs8s++Sfvej+rw,
musl-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rich Felker,
cferris-hpIqsD4AKlfQT0dZR+AlfA, enh-hpIqsD4AKlfQT0dZR+AlfA,
Joseph S. Myers
In-Reply-To: <5565E2AC.3070306-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
On Wednesday 27 May 2015 08:28:44 H. Peter Anvin wrote:
> On 05/18/2015 02:53 AM, Arnd Bergmann wrote:
> > In the patch series I posted recently [1], I introduce new system calls to deal
> > with modified data structures, but left the question open on how these should
> > be best accessed from libc. The patches introduce a new __kernel_time64_t type
> > and based on that five new data structured: struct __kernel_timespec,
> > struct __kernel_itimerspec, struct __kernel_stat, struct __kernel_rusage,
> > and struct __kernel_timex. This works fine for the case when all libc
> > implementations provide their own definitions to user space, but not for
> > the simplest case (e.g. klibc) where the user-visible structures come directly
> > from the kernel uapi headers.
> >
> > I still don't know what model the various libc developers prefer, so here is
> > an alternative approach, as a patch on top of the previous series:
> >
> > Now, we rename the original structures to struct __old_kernel_*, and use a
> > macro to define either the __old_kernel_* or the __kernel_* structure name
> > to the name we actually want in user space, based on a __KERNEL_TIME_BITS
> > macro that can be set to either 32 or 64 for 32-bit architectures by
> > the libc. Depending on that macro, the compiler will either see one
> > of these combinations (for each of the five structures):
> >
> > a) __BITS_PER_LONG == 32 && __KERNEL_TIME_BITS == 32:
> >
> > struct timespec based on 32-bit __kernel_time_t
> > struct __kernel_timespec based on 64-bit __kernel_time64_t
> >
> > b) __BITS_PER_LONG == 64 && __KERNEL_TIME_BITS == 64:
> >
> > struct timespec based on 64-bit __kernel_time_t
> > struct __kernel_timespec based on 64-bit __kernel_time64_t
> >
> > c) __BITS_PER_LONG == 32 && __KERNEL_TIME_BITS == 64:
> >
> > struct __old_kernel_timespec based on 32-bit __kernel_time_t
> > struct timespec based on 64-bit __kernel_time64_t
> >
> > Would this work for everyone? Any alternative suggestions?
> >
>
> It seems to work, except I don't really understand why there is a
> difference between (b) and (c).
I tried to keep a) and b) similar, and change nothing other than the
introduction of the extra __kernel_time64_t and __kernel_timespec
here, for the case of __KERNEL_TIME_BITS == __BITS_PER_LONG, in order
to minimize the risk of introducing regressions for existing libc
builds against new kernel headers.
Case c) is the only one that actually changes the existing definitions,
which makes it different from both a) and b).
> I also have no problem just having klibc contain its own definitions of
> these structures, as long as one can prevent the kernel from defining them.
Ok, good to know. If that works for all libc implementations, we
could take a simpler approach and just define __old_kernel_timespec
plus __kernel_timespec but not timespec (and respectively for the
other structures) when __KERNEL_TIME_BITS == 64 is set, and leave
the real user space definitions up to libc (which can then
add the #defines or not if they want to).
Arnd
^ permalink raw reply
* Re: [PATCH v2 1/2] capabilities: Ambient capabilities
From: Andy Lutomirski @ 2015-05-27 22:35 UTC (permalink / raw)
To: Christoph Lameter
Cc: Jarkko Sakkinen, Ted Ts'o, Andrew G. Morgan, Andrew Morton,
Serge Hallyn, Michael Kerrisk, Mimi Zohar, Linux API,
Austin S Hemmelgarn, linux-security-module, Aaron Jones, LKML,
Serge Hallyn, Markku Savela, Kees Cook, Jonathan Corbet
In-Reply-To: <CALCETrU5B3aR0s9c7qsktLJ4hiBfs0+LqyVNmq1ZVNfF0zFECQ@mail.gmail.com>
On Mon, May 18, 2015 at 12:43 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On May 15, 2015 11:31 PM, "Christoph Lameter" <cl@linux.com> wrote:
>>
>> It would be best to start a complete new thread about this. You
>> replied to earlier posts about ambient capabilities and
>> people may not see it as a new release.
>>
>> > pA obeys the invariant that no bit can ever be set in pA if it is
>> > not set in both pP and pI. Dropping a bit from pP or pI drops that
>> > bit from pA. This ensures that existing programs that try to drop
>> > capabilities still do so, with a complication. Because capability
>>
>> Ok that is a good improvement.
>>
>> > inheritance is so broken, setting KEEPCAPS, using setresuid to
>> > switch to nonroot uids, or calling execve effectively drops
>> > capabilities. Therefore, setresuid from root to nonroot
>> > conditionally clears pA unless SECBIT_NO_SETUID_FIXUP is set.
>> > Processes that don't like this can re-add bits to pA afterwards.
>> >
>> > The capability evolution rules are changed:
>> >
>> > pA' = (file caps or setuid or setgid ? 0 : pA)
>> > pP' = (X & fP) | (pI & fI) | pA'
>> > pI' = pI
>> > pE' = (fE ? pP' : pA')
>>
>> Isnt this equal to
>>
>> pE' = (fE & pP') | pA'
>>
>> which does not require conditionals and is symmetric to how pP' is
>> calculated. Your formula seems to indicate that pA' bits are not set if
>> fE is set. However they are already set unconditionally in pP' regardless.
>> This makes it more explicit I think. And I thought we are dealing with
>> bitmask arithmetic here?
>
> I think you're right, except that fE is a Boolean, not a bit mask, so
> fE | pP' is an odd thing to talk about.
>
> We could say (fE ? pP' : 0) | pA', which could simplify the code a tiny bit.
It turns out that would be almost a pure addition of code, so I'll
leave it as is.
--Andy
^ permalink raw reply
* Re: [PATCH v2 1/2] capabilities: Ambient capabilities
From: Andy Lutomirski @ 2015-05-27 22:44 UTC (permalink / raw)
To: Serge Hallyn
Cc: Andy Lutomirski, Serge Hallyn, Andrew Morton, Jarkko Sakkinen,
Ted Ts'o, Andrew G. Morgan, Linux API, Mimi Zohar,
Michael Kerrisk, Austin S Hemmelgarn, linux-security-module,
Aaron Jones, Serge Hallyn, LKML, Markku Savela, Kees Cook,
Jonathan Corbet, Christoph Lameter
In-Reply-To: <20150523193705.GA30563-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
On Sat, May 23, 2015 at 12:37 PM, Serge Hallyn <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org> wrote:
> Thanks very much, Andy. Comments and ack below.
>
> Quoting Andy Lutomirski (luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org):
>> ===== The problem =====
>>
>> Capability inheritance is basically useless.
>>
>> If you aren't root and you execute an ordinary binary, fI is zero,
>> so your capabilities have no effect whatsoever on pP'. This means
>> that you can't usefully execute a helper process or a shell command
>> with elevated capabilities if you aren't root.
>>
>> On current kernels, you can sort of work around this by setting fI
>> to the full set for most or all non-setuid executable files. This
>> causes pP' = pI for nonroot, and inheritance works. No one does
>> this because it's a PITA and it isn't even supported on most
>> filesystems.
>>
>> If you try this, you'll discover that every nonroot program ends up
>> with secure exec rules, breaking many things.
>
> PI would have worked great if most programs wanting privilege were
> self-contained and compiled. Shell scripts and lots of fork+execing
> make pI [much less useful] [completely useless]. See also golang's
> predisposition to fork+exec.
Agreed.
>
>> This is a problem that has bitten many people who have tried to use
>> capabilities for anything useful.
>>
>> ===== The proposed change =====
>>
>> This patch adds a fifth capability mask called the ambient mask
>> (pA). pA does what pI should have done.
>
> Or at least what most people want it to do.
>
>> pA obeys the invariant that no bit can ever be set in pA if it is
>> not set in both pP and pI. Dropping a bit from pP or pI drops that
>> bit from pA. This ensures that existing programs that try to drop
>> capabilities still do so, with a complication. Because capability
>> inheritance is so broken, setting KEEPCAPS, using setresuid to
>
> Sorry, did you mean "... setting KEEPCAPS and then either using
> setresuid to a nonroot uid or calling execve ..." ?
So, I actually meant:
Because capability inheritance is so broken, setting KEEPCAPS, using
setresuid to switch to nonroot uids, and then calling execve
effectively drops capabilities.
Fixed.
>
>> switch to nonroot uids, or calling execve effectively drops
>> capabilities. Therefore, setresuid from root to nonroot
>> conditionally clears pA unless SECBIT_NO_SETUID_FIXUP is set.
>> Processes that don't like this can re-add bits to pA afterwards.
>>
>> The capability evolution rules are changed:
>>
>> pA' = (file caps or setuid or setgid ? 0 : pA)
>> pP' = (X & fP) | (pI & fI) | pA'
>> pI' = pI
>> pE' = (fE ? pP' : pA')
>> X is unchanged
>>
>> If you are nonroot but you have a capability, you can add it to pA.
>> If you do so, your children get that capability in pA, pP, and pE.
>> For example, you can set pA = CAP_NET_BIND_SERVICE, and your
>> children can automatically bind low-numbered ports. Hallelujah!
>>
>> Unprivileged users can create user namespaces, map themselves to a
>> nonzero uid, and create both privileged (relative to their
>> namespace) and unprivileged process trees. This is currently more
>> or less impossible. Hallelujah!
>>
>> You cannot use pA to try to subvert a setuid, setgid, or file-capped
>> program: if you execute any such program, pA gets cleared and the
>> resulting evolution rules are unchanged by this patch.
>
> Christoph, just to be sure, is this ^ going to suffice for you?
>
> Seems like it should since any program which is setuid-root, i.e.
> passwd, isn't likely to be designed to exec other programs.
>
>> Users with nonzero pA are unlikely to unintentionally leak that
>> capability. If they run programs that try to drop privileges,
>> dropping privileges will still work.
>>
>> It's worth noting that the degree of paranoia in this patch could
>> possibly be reduced without causing serious problems. Specifically,
>> if we allowed pA to persist across executing non-pA-aware setuid
>> binaries and across setresuid, then, naively, the only capabilities
>> that could leak as a result would be the capabilities in pA, and any
>> attacker *already* has those capabilities. This would make me
>> nervous, though -- setuid binaries that tried to privilege-separate
>> might fail to do so, and putting CAP_DAC_READ_SEARCH or
>> CAP_DAC_OVERRIDE into pA could have unexpected side effects.
>> (Whether these unexpected side effects would be exploitable is an
>> open question.) I've therefore taken the more paranoid route.
>
> I'm definitely open to revisiting this question after the current
> patch has been in use for awhile.
Noted in the changelog for v3.
>
>> An alternative would be to require PR_SET_NO_NEW_PRIVS before
>> setting ambient capabilities. I think that this would be annoying
>> and would make granting otherwise unprivileged users minor ambient
>> capabilities (CAP_NET_BIND_SERVICE or CAP_NET_RAW for example) much
>> less useful than it is with this patch.
>>
>> ===== Footnotes =====
>>
>> [1] Files that are missing the "security.capability" xattr or that
>> have unrecognized values for that xattr end up with has_cap set to
>> false. The code that does that appears to be complicated for no
>> good reason.
>>
>> [2] The libcap capability mask parsers and formatters are
>> dangerously misleading and the documentation is flat-out wrong. fE
>> is *not* a mask; it's a single bit. This has probably confused
>> every single person who has tried to use file capabilities.
>>
>> [3] Linux very confusingly processes both the script and the
>> interpreter if applicable, for reasons that elude me. The results
>> from thinking about a script's file capabilities and/or setuid bits
>> are mostly discarded.
>
> Not quite sure what you mean here - setuid and fX are ignored on
> shell scripts, but I think you're saying something less simple is
> going on?
I think that's the effect. What I mean is that, when running a
script, prepare_binprm and hence cap_bprm_set_creds is called twice:
once from do_execveat_common and once from load_script.
Thanks,
Andy
^ permalink raw reply
* Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Greg KH @ 2015-05-27 23:14 UTC (permalink / raw)
To: Winkler, Tomas
Cc: arnd-r2nGTMty4D4@public.gmane.org, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B3C4A2CDF-Jy8z56yoSI/jxeytcECX8bfspsVTdybXVpNB7YpNyf8@public.gmane.org>
On Wed, May 27, 2015 at 05:42:22PM +0000, Winkler, Tomas wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> > Sent: Wednesday, May 27, 2015 20:29
> > To: Winkler, Tomas
> > Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw
> > arrays
> >
> > On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:
> > >
> > > >
> > > > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > > > > In order for mei client devices to use device id based on uuid we
> > > > > have to use common types between user space (file2alias.c).
> > > > > Similarly to vmbus, mei uses raw 16 byte array for that.
> > > > > To leverage on existing infrastructure around uuid_le type
> > > > > defined in uuid.h we add helper macros to handle conversions between
> > > > > raw 16 byte array and uuid_{le,be} types.
> > > >
> > > > You aren't adding a helper macro, you are just redefining the existing
> > > > macros using a different one.
> > >
> > > Not exactly I'm using both the one I've added for device ids and the old one for
> > all the other flows.
> > >
> > > But I can't see why this is needed, what
> > > > does this solve that vmbus and other uses of the existing macros don't
> > > > need? In other words, what makes mei so special that it needs a "lower"
> > > > level macro than every other subsystem?
> > >
> > > It's not special there is actually a lot of code duplication around uuid handling
> > > every subsystem is using their own macros but it can be consolidated around
> > uuid.h
> > >
> > > So vmbus can use that
> > > Instead of
> > > /*
> > > * Network GUID
> > > * {f8615163-df3e-46c5-913f-f2d2f965ed0e}
> > > */
> > > #define HV_NIC_GUID \
> > > .guid = { \
> > > 0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
> > > 0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
> > > }
> > >
> > > The can use the new macro to make it more readable, something in spirit of:
> > >
> > > #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-f2d2f965ed0e)
> >
> > Why the "__" usage here? That signifies a "private" namespace, why add
> > that to the user visible header files?
>
> I take any other suggestion for macro names.
> Not sure how I would reuse the macros if I don't export them both,
> second this can be used also by user space.
But it's not needed at all.
Below is a patch on top of my current tree that makes this patch not
needed. Any objection to me just applying it instead?
> > And are you going to send patches for vmbus and other drivers to fix
> > everything up to use these new macros? Someone has to...
>
> Can be done but I cannot test their code and now I'm busy with splitting the big bus patch :)
Ok, that means no one is ever going to do that work, so it's not a valid
reason to accept such a change. I prefer the patch below.
thanks,
greg k-h
diff --git a/drivers/nfc/mei_phy.h b/drivers/nfc/mei_phy.h
index a51f8f2685cc..fbfa3e61738f 100644
--- a/drivers/nfc/mei_phy.h
+++ b/drivers/nfc/mei_phy.h
@@ -5,7 +5,7 @@
#include <net/nfc/hci.h>
#include <linux/uuid.h>
-#define MEI_NFC_UUID __UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
+#define MEI_NFC_UUID UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
0x94, 0xd4, 0x50, 0x26, 0x67, 0x23, 0x77, 0x5c)
#define MEI_NFC_HEADER_SIZE 10
#define MEI_NFC_MAX_HCI_PAYLOAD 300
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 2d2b2b571d61..048c270822f9 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -614,7 +614,7 @@ struct ipack_device_id {
*/
struct mei_cl_device_id {
char name[MEI_CL_NAME_SIZE];
- __u8 uuid[16];
+ uuid_le uuid;
kernel_ulong_t driver_info;
};
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 62c517f4b592..718b2a29bd43 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -34,6 +34,9 @@ typedef Elf64_Addr kernel_ulong_t;
typedef uint32_t __u32;
typedef uint16_t __u16;
typedef unsigned char __u8;
+typedef struct {
+ __u8 b[16];
+} uuid_le;
/* Big exception to the "don't include kernel headers into userspace, which
* even potentially has different endianness and word sizes, since
@@ -131,13 +134,13 @@ static inline void add_wildcard(char *str)
strcat(str + len, "*");
}
-static inline void add_uuid(char *str, __u8 uuid[16])
+static inline void add_uuid(char *str, uuid_le uuid)
{
int len = strlen(str);
int i;
for (i = 0; i < 16; i++)
- sprintf(str + len + (i << 1), "%02x", uuid[i]);
+ sprintf(str + len + (i << 1), "%02x", uuid.b[i]);
}
/**
^ permalink raw reply related
* Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Greg KH @ 2015-05-27 23:22 UTC (permalink / raw)
To: Winkler, Tomas
Cc: arnd-r2nGTMty4D4@public.gmane.org, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150527231420.GA29446-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Wed, May 27, 2015 at 04:14:20PM -0700, Greg KH wrote:
> On Wed, May 27, 2015 at 05:42:22PM +0000, Winkler, Tomas wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> > > Sent: Wednesday, May 27, 2015 20:29
> > > To: Winkler, Tomas
> > > Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw
> > > arrays
> > >
> > > On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:
> > > >
> > > > >
> > > > > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > > > > > In order for mei client devices to use device id based on uuid we
> > > > > > have to use common types between user space (file2alias.c).
> > > > > > Similarly to vmbus, mei uses raw 16 byte array for that.
> > > > > > To leverage on existing infrastructure around uuid_le type
> > > > > > defined in uuid.h we add helper macros to handle conversions between
> > > > > > raw 16 byte array and uuid_{le,be} types.
> > > > >
> > > > > You aren't adding a helper macro, you are just redefining the existing
> > > > > macros using a different one.
> > > >
> > > > Not exactly I'm using both the one I've added for device ids and the old one for
> > > all the other flows.
> > > >
> > > > But I can't see why this is needed, what
> > > > > does this solve that vmbus and other uses of the existing macros don't
> > > > > need? In other words, what makes mei so special that it needs a "lower"
> > > > > level macro than every other subsystem?
> > > >
> > > > It's not special there is actually a lot of code duplication around uuid handling
> > > > every subsystem is using their own macros but it can be consolidated around
> > > uuid.h
> > > >
> > > > So vmbus can use that
> > > > Instead of
> > > > /*
> > > > * Network GUID
> > > > * {f8615163-df3e-46c5-913f-f2d2f965ed0e}
> > > > */
> > > > #define HV_NIC_GUID \
> > > > .guid = { \
> > > > 0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
> > > > 0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
> > > > }
> > > >
> > > > The can use the new macro to make it more readable, something in spirit of:
> > > >
> > > > #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-f2d2f965ed0e)
> > >
> > > Why the "__" usage here? That signifies a "private" namespace, why add
> > > that to the user visible header files?
> >
> > I take any other suggestion for macro names.
> > Not sure how I would reuse the macros if I don't export them both,
> > second this can be used also by user space.
>
> But it's not needed at all.
>
> Below is a patch on top of my current tree that makes this patch not
> needed. Any objection to me just applying it instead?
>
> > > And are you going to send patches for vmbus and other drivers to fix
> > > everything up to use these new macros? Someone has to...
> >
> > Can be done but I cannot test their code and now I'm busy with splitting the big bus patch :)
>
> Ok, that means no one is ever going to do that work, so it's not a valid
> reason to accept such a change. I prefer the patch below.
Oops, that didn't build, I forgot one change in the mei core (getting
rid of two casts.) Try this one instead.
thanks,
greg k-h
diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
index de8fd089a8a4..dd52f224027e 100644
--- a/drivers/misc/mei/bus.c
+++ b/drivers/misc/mei/bus.c
@@ -54,9 +54,9 @@ static int mei_cl_device_match(struct device *dev, struct device_driver *drv)
id = driver->id_table;
- while (uuid_le_cmp(NULL_UUID_LE, uuid_le_cast(id->uuid))) {
+ while (uuid_le_cmp(NULL_UUID_LE, id->uuid)) {
- if (!uuid_le_cmp(*uuid, uuid_le_cast(id->uuid))) {
+ if (!uuid_le_cmp(*uuid, id->uuid)) {
if (id->name[0]) {
if (!strncmp(name, id->name, sizeof(id->name)))
return 1;
diff --git a/drivers/nfc/mei_phy.h b/drivers/nfc/mei_phy.h
index a51f8f2685cc..fbfa3e61738f 100644
--- a/drivers/nfc/mei_phy.h
+++ b/drivers/nfc/mei_phy.h
@@ -5,7 +5,7 @@
#include <net/nfc/hci.h>
#include <linux/uuid.h>
-#define MEI_NFC_UUID __UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
+#define MEI_NFC_UUID UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
0x94, 0xd4, 0x50, 0x26, 0x67, 0x23, 0x77, 0x5c)
#define MEI_NFC_HEADER_SIZE 10
#define MEI_NFC_MAX_HCI_PAYLOAD 300
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 2d2b2b571d61..048c270822f9 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -614,7 +614,7 @@ struct ipack_device_id {
*/
struct mei_cl_device_id {
char name[MEI_CL_NAME_SIZE];
- __u8 uuid[16];
+ uuid_le uuid;
kernel_ulong_t driver_info;
};
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 62c517f4b592..718b2a29bd43 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -34,6 +34,9 @@ typedef Elf64_Addr kernel_ulong_t;
typedef uint32_t __u32;
typedef uint16_t __u16;
typedef unsigned char __u8;
+typedef struct {
+ __u8 b[16];
+} uuid_le;
/* Big exception to the "don't include kernel headers into userspace, which
* even potentially has different endianness and word sizes, since
@@ -131,13 +134,13 @@ static inline void add_wildcard(char *str)
strcat(str + len, "*");
}
-static inline void add_uuid(char *str, __u8 uuid[16])
+static inline void add_uuid(char *str, uuid_le uuid)
{
int len = strlen(str);
int i;
for (i = 0; i < 16; i++)
- sprintf(str + len + (i << 1), "%02x", uuid[i]);
+ sprintf(str + len + (i << 1), "%02x", uuid.b[i]);
}
/**
^ permalink raw reply related
* RE: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Winkler, Tomas @ 2015-05-27 23:24 UTC (permalink / raw)
To: Greg KH
Cc: arnd-r2nGTMty4D4@public.gmane.org, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150527231420.GA29446-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> > > Sent: Wednesday, May 27, 2015 20:29
> > > To: Winkler, Tomas
> > > Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw
> > > arrays
> > >
> > > On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:
> > > >
> > > > >
> > > > > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > > > > > In order for mei client devices to use device id based on uuid we
> > > > > > have to use common types between user space (file2alias.c).
> > > > > > Similarly to vmbus, mei uses raw 16 byte array for that.
> > > > > > To leverage on existing infrastructure around uuid_le type
> > > > > > defined in uuid.h we add helper macros to handle conversions between
> > > > > > raw 16 byte array and uuid_{le,be} types.
> > > > >
> > > > > You aren't adding a helper macro, you are just redefining the existing
> > > > > macros using a different one.
> > > >
> > > > Not exactly I'm using both the one I've added for device ids and the old one
> for
> > > all the other flows.
> > > >
> > > > But I can't see why this is needed, what
> > > > > does this solve that vmbus and other uses of the existing macros don't
> > > > > need? In other words, what makes mei so special that it needs a "lower"
> > > > > level macro than every other subsystem?
> > > >
> > > > It's not special there is actually a lot of code duplication around uuid
> handling
> > > > every subsystem is using their own macros but it can be consolidated
> around
> > > uuid.h
> > > >
> > > > So vmbus can use that
> > > > Instead of
> > > > /*
> > > > * Network GUID
> > > > * {f8615163-df3e-46c5-913f-f2d2f965ed0e}
> > > > */
> > > > #define HV_NIC_GUID \
> > > > .guid = { \
> > > > 0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
> > > > 0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
> > > > }
> > > >
> > > > The can use the new macro to make it more readable, something in spirit of:
> > > >
> > > > #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-f2d2f965ed0e)
> > >
> > > Why the "__" usage here? That signifies a "private" namespace, why add
> > > that to the user visible header files?
> >
> > I take any other suggestion for macro names.
> > Not sure how I would reuse the macros if I don't export them both,
> > second this can be used also by user space.
>
> But it's not needed at all.
>
> Below is a patch on top of my current tree that makes this patch not
> needed. Any objection to me just applying it instead?
Looks like it might work I even like it more as I can drop the conversions between raw type and uuid, but I have to look if more modification is needed in mei.
Just need to make sure these types are always in sync.
> > > And are you going to send patches for vmbus and other drivers to fix
> > > everything up to use these new macros? Someone has to...
> >
> > Can be done but I cannot test their code and now I'm busy with splitting the big
> bus patch :)
>
> Ok, that means no one is ever going to do that work, so it's not a valid
> reason to accept such a change. I prefer the patch below
You may also here leverage for vmbus to this direction so this point is semi valid.
>
> thanks,
>
> greg k-h
>
> diff --git a/drivers/nfc/mei_phy.h b/drivers/nfc/mei_phy.h
> index a51f8f2685cc..fbfa3e61738f 100644
> --- a/drivers/nfc/mei_phy.h
> +++ b/drivers/nfc/mei_phy.h
> @@ -5,7 +5,7 @@
> #include <net/nfc/hci.h>
> #include <linux/uuid.h>
>
> -#define MEI_NFC_UUID __UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
> +#define MEI_NFC_UUID UUID_LE(0x0bb17a78, 0x2a8e, 0x4c50, \
> 0x94, 0xd4, 0x50, 0x26, 0x67, 0x23, 0x77, 0x5c)
> #define MEI_NFC_HEADER_SIZE 10
> #define MEI_NFC_MAX_HCI_PAYLOAD 300
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 2d2b2b571d61..048c270822f9 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -614,7 +614,7 @@ struct ipack_device_id {
> */
> struct mei_cl_device_id {
> char name[MEI_CL_NAME_SIZE];
> - __u8 uuid[16];
> + uuid_le uuid;
> kernel_ulong_t driver_info;
> };
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 62c517f4b592..718b2a29bd43 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -34,6 +34,9 @@ typedef Elf64_Addr kernel_ulong_t;
> typedef uint32_t __u32;
> typedef uint16_t __u16;
> typedef unsigned char __u8;
> +typedef struct {
> + __u8 b[16];
> +} uuid_le;
>
> /* Big exception to the "don't include kernel headers into userspace, which
> * even potentially has different endianness and word sizes, since
> @@ -131,13 +134,13 @@ static inline void add_wildcard(char *str)
> strcat(str + len, "*");
> }
>
> -static inline void add_uuid(char *str, __u8 uuid[16])
> +static inline void add_uuid(char *str, uuid_le uuid)
> {
> int len = strlen(str);
> int i;
>
> for (i = 0; i < 16; i++)
> - sprintf(str + len + (i << 1), "%02x", uuid[i]);
> + sprintf(str + len + (i << 1), "%02x", uuid.b[i]);
> }
>
> /**
^ permalink raw reply
* RE: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw arrays
From: Winkler, Tomas @ 2015-05-27 23:25 UTC (permalink / raw)
To: Greg KH
Cc: arnd-r2nGTMty4D4@public.gmane.org, Stephen Rothwell,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20150527232254.GA8737-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
> -----Original Message-----
> From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> Sent: Thursday, May 28, 2015 02:23
> To: Winkler, Tomas
> Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning raw
> arrays
>
> On Wed, May 27, 2015 at 04:14:20PM -0700, Greg KH wrote:
> > On Wed, May 27, 2015 at 05:42:22PM +0000, Winkler, Tomas wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org]
> > > > Sent: Wednesday, May 27, 2015 20:29
> > > > To: Winkler, Tomas
> > > > Cc: arnd-r2nGTMty4D4@public.gmane.org; Stephen Rothwell; linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > Subject: Re: [char-misc-next 04/11 V2] uuid: extract macros for assigning
> raw
> > > > arrays
> > > >
> > > > On Wed, May 27, 2015 at 05:24:01PM +0000, Winkler, Tomas wrote:
> > > > >
> > > > > >
> > > > > > On Wed, May 27, 2015 at 06:42:13PM +0300, Tomas Winkler wrote:
> > > > > > > In order for mei client devices to use device id based on uuid we
> > > > > > > have to use common types between user space (file2alias.c).
> > > > > > > Similarly to vmbus, mei uses raw 16 byte array for that.
> > > > > > > To leverage on existing infrastructure around uuid_le type
> > > > > > > defined in uuid.h we add helper macros to handle conversions
> between
> > > > > > > raw 16 byte array and uuid_{le,be} types.
> > > > > >
> > > > > > You aren't adding a helper macro, you are just redefining the existing
> > > > > > macros using a different one.
> > > > >
> > > > > Not exactly I'm using both the one I've added for device ids and the old
> one for
> > > > all the other flows.
> > > > >
> > > > > But I can't see why this is needed, what
> > > > > > does this solve that vmbus and other uses of the existing macros don't
> > > > > > need? In other words, what makes mei so special that it needs a
> "lower"
> > > > > > level macro than every other subsystem?
> > > > >
> > > > > It's not special there is actually a lot of code duplication around uuid
> handling
> > > > > every subsystem is using their own macros but it can be consolidated
> around
> > > > uuid.h
> > > > >
> > > > > So vmbus can use that
> > > > > Instead of
> > > > > /*
> > > > > * Network GUID
> > > > > * {f8615163-df3e-46c5-913f-f2d2f965ed0e}
> > > > > */
> > > > > #define HV_NIC_GUID \
> > > > > .guid = { \
> > > > > 0x63, 0x51, 0x61, 0xf8, 0x3e, 0xdf, 0xc5, 0x46, \
> > > > > 0x91, 0x3f, 0xf2, 0xd2, 0xf9, 0x65, 0xed, 0x0e \
> > > > > }
> > > > >
> > > > > The can use the new macro to make it more readable, something in spirit
> of:
> > > > >
> > > > > #define HV_NIC_GUID __UUID_LE(f8615163-df3e-46c5-913f-
> f2d2f965ed0e)
> > > >
> > > > Why the "__" usage here? That signifies a "private" namespace, why add
> > > > that to the user visible header files?
> > >
> > > I take any other suggestion for macro names.
> > > Not sure how I would reuse the macros if I don't export them both,
> > > second this can be used also by user space.
> >
> > But it's not needed at all.
> >
> > Below is a patch on top of my current tree that makes this patch not
> > needed. Any objection to me just applying it instead?
> >
> > > > And are you going to send patches for vmbus and other drivers to fix
> > > > everything up to use these new macros? Someone has to...
> > >
> > > Can be done but I cannot test their code and now I'm busy with splitting the
> big bus patch :)
> >
> > Ok, that means no one is ever going to do that work, so it's not a valid
> > reason to accept such a change. I prefer the patch below.
>
> Oops, that didn't build, I forgot one change in the mei core (getting
> rid of two casts.) Try this one instead.
>
> thanks,
>
> greg k-h
>
>
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index de8fd089a8a4..dd52f224027e 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -54,9 +54,9 @@ static int mei_cl_device_match(struct device *dev, struct
> device_driver *drv)
>
> id = driver->id_table;
>
> - while (uuid_le_cmp(NULL_UUID_LE, uuid_le_cast(id->uuid))) {
> + while (uuid_le_cmp(NULL_UUID_LE, id->uuid)) {
>
> - if (!uuid_le_cmp(*uuid, uuid_le_cast(id->uuid))) {
> + if (!uuid_le_cmp(*uuid, id->uuid)) {
> if (id->name[0]) {
> if (!strncmp(name, id->name, sizeof(id->name)))
> return 1;
You can also drop the uuid_le_cast function
Tomas
^ permalink raw reply
* [PATCH v3 0/2] capabilities: Ambient capability patchset
From: Andy Lutomirski @ 2015-05-27 23:47 UTC (permalink / raw)
To: Serge Hallyn, Andrew Morton, James Morris
Cc: Jarkko Sakkinen, Andrew Lutomirski, Ted Ts'o,
Andrew G. Morgan, Linux API, Mimi Zohar, Michael Kerrisk,
Austin S Hemmelgarn, linux-security-module, Aaron Jones,
Serge Hallyn, LKML, Markku Savela, Kees Cook, Jonathan Corbet
This adds ambient capabilities. See the individual patch changelogs
for details.
Preliminary userspace code is here:
https://git.kernel.org/cgit/linux/kernel/git/luto/util-linux-playground.git/commit/?h=cap_ambient&id=7f5afbd175d2
James or akpm, I think this is ready for 4.2. Can one of you queue it up?
Thanks,
Andy
Changes from v2:
- Improve the patch 1 changelog a bit.
- Add acks.
- Add comment clarifying the pE' rule.
- s/except/expect
Changes from v1:
- Lots of cleanups to the ambient cap code.
- The securebit is new.
Andy Lutomirski (2):
capabilities: Ambient capabilities
capabilities: Add a securebit to disable PR_CAP_AMBIENT_RAISE
fs/proc/array.c | 5 ++-
include/linux/cred.h | 8 ++++
include/uapi/linux/prctl.h | 6 +++
include/uapi/linux/securebits.h | 11 ++++-
kernel/user_namespace.c | 1 +
security/commoncap.c | 92 ++++++++++++++++++++++++++++++++++++-----
security/keys/process_keys.c | 1 +
7 files changed, 112 insertions(+), 12 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH v3 1/2] capabilities: Ambient capabilities
From: Andy Lutomirski @ 2015-05-27 23:47 UTC (permalink / raw)
To: Serge Hallyn, Andrew Morton, James Morris
Cc: Jarkko Sakkinen, Andrew Lutomirski, Ted Ts'o,
Andrew G. Morgan, Linux API, Mimi Zohar, Michael Kerrisk,
Austin S Hemmelgarn, linux-security-module, Aaron Jones,
Serge Hallyn, LKML, Markku Savela, Kees Cook, Jonathan Corbet,
Christoph Lameter, Andy Lutomirski
In-Reply-To: <cover.1432770087.git.luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Credit where credit is due: this idea comes from Christoph Lameter
with a lot of valuable input from Serge Hallyn. This patch is
heavily based on Christoph's patch.
===== The status quo =====
On Linux, there are a number of capabilities defined by the kernel.
To perform various privileged tasks, processes can wield
capabilities that they hold.
Each task has four capability masks: effective (pE), permitted (pP),
inheritable (pI), and a bounding set (X). When the kernel checks
for a capability, it checks pE. The other capability masks serve to
modify what capabilities can be in pE.
Any task can remove capabilities from pE, pP, or pI at any time. If
a task has a capability in pP, it can add that capability to pE
and/or pI. If a task has CAP_SETPCAP, then it can add any
capability to pI, and it can remove capabilities from X.
Tasks are not the only things that can have capabilities; files can
also have capabilities. A file can have no capabilty information at
all [1]. If a file has capability information, then it has a
permitted mask (fP) and an inheritable mask (fI) as well as a single
effective bit (fE) [2]. File capabilities modify the capabilities
of tasks that execve(2) them.
A task that successfully calls execve has its capabilities modified
for the file ultimately being excecuted (i.e. the binary itself if
that binary is ELF or for the interpreter if the binary is a
script.) [3] In the capability evolution rules, for each mask Z, pZ
represents the old value and pZ' represents the new value. The
rules are:
pP' = (X & fP) | (pI & fI)
pI' = pI
pE' = (fE ? pP' : 0)
X is unchanged
For setuid binaries, fP, fI, and fE are modified by a moderately
complicated set of rules that emulate POSIX behavior. Similarly, if
euid == 0 or ruid == 0, then fP, fI, and fE are modified differently
(primary, fP and fI usually end up being the full set). For nonroot
users executing binaries with neither setuid nor file caps, fI and
fP are empty and fE is false.
As an extra complication, if you execute a process as nonroot and fE
is set, then the "secure exec" rules are in effect: AT_SECURE gets
set, LD_PRELOAD doesn't work, etc.
This is rather messy. We've learned that making any changes is
dangerous, though: if a new kernel version allows an unprivileged
program to change its security state in a way that persists cross
execution of a setuid program or a program with file caps, this
persistent state is surprisingly likely to allow setuid or
file-capped programs to be exploited for privilege escalation.
===== The problem =====
Capability inheritance is basically useless.
If you aren't root and you execute an ordinary binary, fI is zero,
so your capabilities have no effect whatsoever on pP'. This means
that you can't usefully execute a helper process or a shell command
with elevated capabilities if you aren't root.
On current kernels, you can sort of work around this by setting fI
to the full set for most or all non-setuid executable files. This
causes pP' = pI for nonroot, and inheritance works. No one does
this because it's a PITA and it isn't even supported on most
filesystems.
If you try this, you'll discover that every nonroot program ends up
with secure exec rules, breaking many things.
This is a problem that has bitten many people who have tried to use
capabilities for anything useful.
===== The proposed change =====
This patch adds a fifth capability mask called the ambient mask
(pA). pA does what most people expect pI to do.
pA obeys the invariant that no bit can ever be set in pA if it is
not set in both pP and pI. Dropping a bit from pP or pI drops that
bit from pA. This ensures that existing programs that try to drop
capabilities still do so, with a complication. Because capability
inheritance is so broken, setting KEEPCAPS, using setresuid to
switch to nonroot uids, and then calling execve effectively drops
capabilities. Therefore, setresuid from root to nonroot
conditionally clears pA unless SECBIT_NO_SETUID_FIXUP is set.
Processes that don't like this can re-add bits to pA afterwards.
The capability evolution rules are changed:
pA' = (file caps or setuid or setgid ? 0 : pA)
pP' = (X & fP) | (pI & fI) | pA'
pI' = pI
pE' = (fE ? pP' : pA')
X is unchanged
If you are nonroot but you have a capability, you can add it to pA.
If you do so, your children get that capability in pA, pP, and pE.
For example, you can set pA = CAP_NET_BIND_SERVICE, and your
children can automatically bind low-numbered ports. Hallelujah!
Unprivileged users can create user namespaces, map themselves to a
nonzero uid, and create both privileged (relative to their
namespace) and unprivileged process trees. This is currently more
or less impossible. Hallelujah!
You cannot use pA to try to subvert a setuid, setgid, or file-capped
program: if you execute any such program, pA gets cleared and the
resulting evolution rules are unchanged by this patch.
Users with nonzero pA are unlikely to unintentionally leak that
capability. If they run programs that try to drop privileges,
dropping privileges will still work.
It's worth noting that the degree of paranoia in this patch could
possibly be reduced without causing serious problems. Specifically,
if we allowed pA to persist across executing non-pA-aware setuid
binaries and across setresuid, then, naively, the only capabilities
that could leak as a result would be the capabilities in pA, and any
attacker *already* has those capabilities. This would make me
nervous, though -- setuid binaries that tried to privilege-separate
might fail to do so, and putting CAP_DAC_READ_SEARCH or
CAP_DAC_OVERRIDE into pA could have unexpected side effects.
(Whether these unexpected side effects would be exploitable is an
open question.) I've therefore taken the more paranoid route. We
can revisit this later.
An alternative would be to require PR_SET_NO_NEW_PRIVS before
setting ambient capabilities. I think that this would be annoying
and would make granting otherwise unprivileged users minor ambient
capabilities (CAP_NET_BIND_SERVICE or CAP_NET_RAW for example) much
less useful than it is with this patch.
===== Footnotes =====
[1] Files that are missing the "security.capability" xattr or that
have unrecognized values for that xattr end up with has_cap set to
false. The code that does that appears to be complicated for no
good reason.
[2] The libcap capability mask parsers and formatters are
dangerously misleading and the documentation is flat-out wrong. fE
is *not* a mask; it's a single bit. This has probably confused
every single person who has tried to use file capabilities.
[3] Linux very confusingly processes both the script and the
interpreter if applicable, for reasons that elude me. The results
from thinking about a script's file capabilities and/or setuid bits
are mostly discarded.
Acked-by: Serge E. Hallyn <serge.hallyn-GeWIH/nMZzLQT0dZR+AlfA@public.gmane.org>
Cc: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
Cc: Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>
Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
Cc: Aaron Jones <aaronmdjones-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: Ted Ts'o <tytso-3s7WtUTddSA@public.gmane.org>
Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org
Cc: Andrew G. Morgan <morgan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mimi Zohar <zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: Austin S Hemmelgarn <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Markku Savela <msa-kXoF896ld44xHbG02/KK1g@public.gmane.org>
Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Michael Kerrisk <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> # Original author
Signed-off-by: Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
fs/proc/array.c | 5 ++-
include/linux/cred.h | 8 ++++
include/uapi/linux/prctl.h | 6 +++
kernel/user_namespace.c | 1 +
security/commoncap.c | 91 +++++++++++++++++++++++++++++++++++++++-----
security/keys/process_keys.c | 1 +
6 files changed, 101 insertions(+), 11 deletions(-)
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 1295a00ca316..bc15356d6551 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -282,7 +282,8 @@ static void render_cap_t(struct seq_file *m, const char *header,
static inline void task_cap(struct seq_file *m, struct task_struct *p)
{
const struct cred *cred;
- kernel_cap_t cap_inheritable, cap_permitted, cap_effective, cap_bset;
+ kernel_cap_t cap_inheritable, cap_permitted, cap_effective,
+ cap_bset, cap_ambient;
rcu_read_lock();
cred = __task_cred(p);
@@ -290,12 +291,14 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p)
cap_permitted = cred->cap_permitted;
cap_effective = cred->cap_effective;
cap_bset = cred->cap_bset;
+ cap_ambient = cred->cap_ambient;
rcu_read_unlock();
render_cap_t(m, "CapInh:\t", &cap_inheritable);
render_cap_t(m, "CapPrm:\t", &cap_permitted);
render_cap_t(m, "CapEff:\t", &cap_effective);
render_cap_t(m, "CapBnd:\t", &cap_bset);
+ render_cap_t(m, "CapAmb:\t", &cap_ambient);
}
static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 2fb2ca2127ed..05178874e771 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -122,6 +122,7 @@ struct cred {
kernel_cap_t cap_permitted; /* caps we're permitted */
kernel_cap_t cap_effective; /* caps we can actually use */
kernel_cap_t cap_bset; /* capability bounding set */
+ kernel_cap_t cap_ambient; /* Ambient capability set */
#ifdef CONFIG_KEYS
unsigned char jit_keyring; /* default keyring to attach requested
* keys to */
@@ -197,6 +198,13 @@ static inline void validate_process_creds(void)
}
#endif
+static inline bool cap_ambient_invariant_ok(const struct cred *cred)
+{
+ return cap_issubset(cred->cap_ambient,
+ cap_intersect(cred->cap_permitted,
+ cred->cap_inheritable));
+}
+
/**
* get_new_cred - Get a reference on a new set of credentials
* @cred: The new credentials to reference
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 31891d9535e2..65407f867e82 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -190,4 +190,10 @@ struct prctl_mm_map {
# define PR_FP_MODE_FR (1 << 0) /* 64b FP registers */
# define PR_FP_MODE_FRE (1 << 1) /* 32b compatibility */
+/* Control the ambient capability set */
+#define PR_CAP_AMBIENT 47
+# define PR_CAP_AMBIENT_GET 1
+# define PR_CAP_AMBIENT_RAISE 2
+# define PR_CAP_AMBIENT_LOWER 3
+
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 4109f8320684..dab0f808235a 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -39,6 +39,7 @@ static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
cred->cap_inheritable = CAP_EMPTY_SET;
cred->cap_permitted = CAP_FULL_SET;
cred->cap_effective = CAP_FULL_SET;
+ cred->cap_ambient = CAP_EMPTY_SET;
cred->cap_bset = CAP_FULL_SET;
#ifdef CONFIG_KEYS
key_put(cred->request_key_auth);
diff --git a/security/commoncap.c b/security/commoncap.c
index f66713bd7450..835a7584f7ea 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -272,6 +272,16 @@ int cap_capset(struct cred *new,
new->cap_effective = *effective;
new->cap_inheritable = *inheritable;
new->cap_permitted = *permitted;
+
+ /*
+ * Mask off ambient bits that are no longer both permitted and
+ * inheritable.
+ */
+ new->cap_ambient = cap_intersect(new->cap_ambient,
+ cap_intersect(*permitted,
+ *inheritable));
+ if (WARN_ON(!cap_ambient_invariant_ok(new)))
+ return -EINVAL;
return 0;
}
@@ -352,6 +362,7 @@ static inline int bprm_caps_from_vfs_caps(struct cpu_vfs_cap_data *caps,
/*
* pP' = (X & fP) | (pI & fI)
+ * The addition of pA' is handled later.
*/
new->cap_permitted.cap[i] =
(new->cap_bset.cap[i] & permitted) |
@@ -479,10 +490,13 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
{
const struct cred *old = current_cred();
struct cred *new = bprm->cred;
- bool effective, has_cap = false;
+ bool effective, has_cap = false, is_setid;
int ret;
kuid_t root_uid;
+ if (WARN_ON(!cap_ambient_invariant_ok(old)))
+ return -EPERM;
+
effective = false;
ret = get_file_caps(bprm, &effective, &has_cap);
if (ret < 0)
@@ -527,8 +541,9 @@ skip:
*
* In addition, if NO_NEW_PRIVS, then ensure we get no new privs.
*/
- if ((!uid_eq(new->euid, old->uid) ||
- !gid_eq(new->egid, old->gid) ||
+ is_setid = !uid_eq(new->euid, old->uid) || !gid_eq(new->egid, old->gid);
+
+ if ((is_setid ||
!cap_issubset(new->cap_permitted, old->cap_permitted)) &&
bprm->unsafe & ~LSM_UNSAFE_PTRACE_CAP) {
/* downgrade; they get no more than they had, and maybe less */
@@ -544,10 +559,28 @@ skip:
new->suid = new->fsuid = new->euid;
new->sgid = new->fsgid = new->egid;
+ /* File caps or setid cancels ambient. */
+ if (has_cap || is_setid)
+ cap_clear(new->cap_ambient);
+
+ /*
+ * Now that we've computed pA', update pP' to give:
+ * pP' = (X & fP) | (pI & fI) | pA'
+ */
+ new->cap_permitted = cap_combine(new->cap_permitted, new->cap_ambient);
+
+ /*
+ * Set pE' = (fE ? pP' : pA'). Because pA' is zero if fE is set,
+ * this is the same as pE' = (fE ? pP' : 0) | pA'.
+ */
if (effective)
new->cap_effective = new->cap_permitted;
else
- cap_clear(new->cap_effective);
+ new->cap_effective = new->cap_ambient;
+
+ if (WARN_ON(!cap_ambient_invariant_ok(new)))
+ return -EPERM;
+
bprm->cap_effective = effective;
/*
@@ -562,7 +595,7 @@ skip:
* Number 1 above might fail if you don't have a full bset, but I think
* that is interesting information to audit.
*/
- if (!cap_isclear(new->cap_effective)) {
+ if (!cap_issubset(new->cap_effective, new->cap_ambient)) {
if (!cap_issubset(CAP_FULL_SET, new->cap_effective) ||
!uid_eq(new->euid, root_uid) || !uid_eq(new->uid, root_uid) ||
issecure(SECURE_NOROOT)) {
@@ -573,6 +606,10 @@ skip:
}
new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
+
+ if (WARN_ON(!cap_ambient_invariant_ok(new)))
+ return -EPERM;
+
return 0;
}
@@ -594,7 +631,7 @@ int cap_bprm_secureexec(struct linux_binprm *bprm)
if (!uid_eq(cred->uid, root_uid)) {
if (bprm->cap_effective)
return 1;
- if (!cap_isclear(cred->cap_permitted))
+ if (!cap_issubset(cred->cap_permitted, cred->cap_ambient))
return 1;
}
@@ -696,10 +733,18 @@ static inline void cap_emulate_setxuid(struct cred *new, const struct cred *old)
uid_eq(old->suid, root_uid)) &&
(!uid_eq(new->uid, root_uid) &&
!uid_eq(new->euid, root_uid) &&
- !uid_eq(new->suid, root_uid)) &&
- !issecure(SECURE_KEEP_CAPS)) {
- cap_clear(new->cap_permitted);
- cap_clear(new->cap_effective);
+ !uid_eq(new->suid, root_uid))) {
+ if (!issecure(SECURE_KEEP_CAPS)) {
+ cap_clear(new->cap_permitted);
+ cap_clear(new->cap_effective);
+ }
+
+ /*
+ * Pre-ambient programs expect setresuid to nonroot followed
+ * by exec to drop capabilities. We should make sure that
+ * this remains the case.
+ */
+ cap_clear(new->cap_ambient);
}
if (uid_eq(old->euid, root_uid) && !uid_eq(new->euid, root_uid))
cap_clear(new->cap_effective);
@@ -929,6 +974,32 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
return commit_creds(new);
+ case PR_CAP_AMBIENT:
+ if (((!cap_valid(arg3)) | arg4 | arg5))
+ return -EINVAL;
+
+ if (arg2 == PR_CAP_AMBIENT_GET) {
+ return !!cap_raised(current_cred()->cap_ambient, arg3);
+ } else if (arg2 != PR_CAP_AMBIENT_RAISE &&
+ arg2 != PR_CAP_AMBIENT_LOWER) {
+ return -EINVAL;
+ } else {
+ if (arg2 == PR_CAP_AMBIENT_RAISE &&
+ (!cap_raised(current_cred()->cap_permitted, arg3) ||
+ !cap_raised(current_cred()->cap_inheritable,
+ arg3)))
+ return -EPERM;
+
+ new = prepare_creds();
+ if (!new)
+ return -ENOMEM;
+ if (arg2 == PR_CAP_AMBIENT_RAISE)
+ cap_raise(new->cap_ambient, arg3);
+ else
+ cap_lower(new->cap_ambient, arg3);
+ return commit_creds(new);
+ }
+
default:
/* No functionality available - continue with default */
return -ENOSYS;
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index bd536cb221e2..43b4cddbf2b3 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -848,6 +848,7 @@ void key_change_session_keyring(struct callback_head *twork)
new->cap_inheritable = old->cap_inheritable;
new->cap_permitted = old->cap_permitted;
new->cap_effective = old->cap_effective;
+ new->cap_ambient = old->cap_ambient;
new->cap_bset = old->cap_bset;
new->jit_keyring = old->jit_keyring;
--
2.1.0
^ permalink raw reply related
* [PATCH v3 2/2] capabilities: Add a securebit to disable PR_CAP_AMBIENT_RAISE
From: Andy Lutomirski @ 2015-05-27 23:47 UTC (permalink / raw)
To: Serge Hallyn, Andrew Morton, James Morris
Cc: Jarkko Sakkinen, Andrew Lutomirski, Ted Ts'o,
Andrew G. Morgan, Linux API, Mimi Zohar, Michael Kerrisk,
Austin S Hemmelgarn, linux-security-module, Aaron Jones,
Serge Hallyn, LKML, Markku Savela, Kees Cook, Jonathan Corbet,
Christoph Lameter, Andy Lutomirski
In-Reply-To: <cover.1432770087.git.luto@kernel.org>
Per Andrew Morgan's request, add a securebit to allow admins to
disable PR_CAP_AMBIENT_RAISE. This securebit will prevent processes
from adding capabilities to their ambient set.
For simplicity, this disables PR_CAP_AMBIENT_RAISE entirely rather
than just disabling setting previously cleared bits.
Acked-By: Andrew G. Morgan <morgan@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Serge Hallyn <serge.hallyn@canonical.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Aaron Jones <aaronmdjones@gmail.com>
CC: Ted Ts'o <tytso@mit.edu>
Cc: linux-security-module@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-api@vger.kernel.org
Cc: akpm@linuxfoundation.org
Cc: Andrew G. Morgan <morgan@kernel.org>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Austin S Hemmelgarn <ahferroin7@gmail.com>
Cc: Markku Savela <msa@moth.iki.fi>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
include/uapi/linux/securebits.h | 11 ++++++++++-
security/commoncap.c | 3 ++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index 985aac9e6bf8..35ac35cef217 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -43,9 +43,18 @@
#define SECBIT_KEEP_CAPS (issecure_mask(SECURE_KEEP_CAPS))
#define SECBIT_KEEP_CAPS_LOCKED (issecure_mask(SECURE_KEEP_CAPS_LOCKED))
+/* When set, a process cannot add new capabilities to its ambient set. */
+#define SECURE_NO_CAP_AMBIENT_RAISE 6
+#define SECURE_NO_CAP_AMBIENT_RAISE_LOCKED 7 /* make bit-6 immutable */
+
+#define SECBIT_NO_CAP_AMBIENT_RAISE (issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
+#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
+ (issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
+
#define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
issecure_mask(SECURE_NO_SETUID_FIXUP) | \
- issecure_mask(SECURE_KEEP_CAPS))
+ issecure_mask(SECURE_KEEP_CAPS) | \
+ issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
#define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)
#endif /* _UAPI_LINUX_SECUREBITS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index 835a7584f7ea..22b7b91c5eae 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -987,7 +987,8 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
if (arg2 == PR_CAP_AMBIENT_RAISE &&
(!cap_raised(current_cred()->cap_permitted, arg3) ||
!cap_raised(current_cred()->cap_inheritable,
- arg3)))
+ arg3) ||
+ issecure(SECURE_NO_CAP_AMBIENT_RAISE)))
return -EPERM;
new = prepare_creds();
--
2.1.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox