Linux userland API discussions
 help / color / mirror / Atom feed
* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: David Drysdale @ 2014-07-27 12:26 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Eric W. Biederman, Julien Tinnes, Al Viro, Paolo Bonzini,
	LSM List, Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
	Meredydd Luff, Christoph Hellwig,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kees Cook,
	Theodore Ts'o, Henrique de Moraes Holschuh,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CALCETrWGvBfrX1W5XaFgJZhKHLf+oac9YrwPR3ppK=deQtu5hA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Jul 25, 2014 at 7:30 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> [new thread because this sort of combines two threads]
>
> There is recent interest in having a way to turn generally-available
> kernel features off.  Maybe we should add a good one so we can stop
> bikeshedding and avoid proliferating dumb interfaces.
>
> Things that might want to be turn-off-able include:
>  - getrandom with GRND_RANDOM [from the getrandom threads]
>  - Any lookup of a non-self pid [from the capsicum thread]
>  - Any lookup of a pid outside the caller thread group [capsicum]
>  - Various architectural things (personal wishlist), e.g.:
>     - RDTSC and userspace HPET access
>     - CPUID?
>     - 32-bit GDT code segments [huge attack surface]
>     - 64-bit GDT code segments [probably pointless]
>
> I would propose a new syscall for this:
>
> long restrict_userspace(int mode, int type, int value, int flags);
>
> mode is RESTRICT_SET, RESTRICT_GET, or RESTRICT_LOCK.
>
> type is RESTRICT_GRND_RANDOM, RESTRICT_PID_SCOPE, RESTRICT_X86_TIMING, etc.
>
> Value is zero if RESTRICT_GET.  Otherwise value is the desired value,
> generally 0 or 1.  For RESTRICT_PID_SCOPE, value would be
> RESTRICT_PID_SCOPE_ANY, RESTRICT_PID_SCOPE_THREADGROUP, or
> RESTRICT_PID_SCOPE_SELF.
>
> flags must be zero.  Someday, someone will propose a thread-sync flag.

Today, me: proposed :-)

> restrict_userspace requires either no_new_privs or CAP_SYS_ADMIN in
> the current user namespace.
>
> Thoughts?
>
> --Andy
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC

^ permalink raw reply

* Re: [RFC PATCH 1/1] ethtool: adding support for multiple slave port configuration
From: John Fastabend @ 2014-07-27 16:09 UTC (permalink / raw)
  To: Ben Hutchings, Mugunthan V N; +Cc: netdev, davem, linux-api, linux-kernel
In-Reply-To: <1406429221.29010.151.camel@deadeye.wl.decadent.org.uk>

On 07/26/2014 07:47 PM, Ben Hutchings wrote:
> On Fri, 2014-07-25 at 17:58 +0530, Mugunthan V N wrote:
>> Some Ethernet Swtich controllers like CPSW in AM335x, TI814x, DRA7x and
>> AM43xx SoCs, Network Coprocessor in AM5K2E0x, Realtek Switch controllers
>> etc has to capability of conneting multiple networks using L2 switching
>> and has multiple phys. With the existing code, ethtool can communicate
>> only to one phy.
>>
>> To enable user to communicate multiple phy connected to single Ethernet
>> Switch controller, intoducing a optional new parameter in Ethtool interface
>> to pass which slave to set/get the phy configuration.
>
> There was some discussion about configuration APIs for hardware/firmware
> bridges earlier this year and I thought there was a consensus for
> assigning a network device to each port.  This would remove the need to
> identify ports within a device.  But I may have misremembered.
>

I like the approach of creating a network device for each port over
having to use ethtool to program/discover them. I am currently looking
at writing management applications for this and IMO it is much easier
to discover and listen for events on network devices vs polling ethtool
and iterating through slave indexs. Also you miss a lot of functionality
that may be useful MTU for example that is not available configured via
ethtool.

One of the sticking points in earlier discussions was how to handle
devices that have limited support for slave devices. When we create a
netdev we expect the stack can bind to it and TX/RX packets which as
I understand is not always possible? (I missed why we couldn't recv the
packets over a switch port though with some skb->dev manipulation). In
this case a feature flag could be used to resolve the feature
dependencies.

.John


-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Theodore Ts'o @ 2014-07-27 21:06 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Eric W. Biederman, Julien Tinnes, David Drysdale, Al Viro,
	Paolo Bonzini, LSM List, Greg Kroah-Hartman, Paul Moore,
	James Morris, Linux API, Meredydd Luff, Christoph Hellwig,
	linux-kernel@vger.kernel.org, Kees Cook,
	Henrique de Moraes Holschuh, linux-crypto
In-Reply-To: <CALCETrWGvBfrX1W5XaFgJZhKHLf+oac9YrwPR3ppK=deQtu5hA@mail.gmail.com>

On Fri, Jul 25, 2014 at 11:30:48AM -0700, Andy Lutomirski wrote:
> 
> There is recent interest in having a way to turn generally-available
> kernel features off.  Maybe we should add a good one so we can stop
> bikeshedding and avoid proliferating dumb interfaces.

I believe the seccomp infrastructure (which is already upstream)
should be able to do most of what you want, at least with respect to
features which are exposed via system calls (which was most of your
list).

It won't cover x86 specific things like restricting RDTSC or CPUID
(and as far as I know you can't intercept the CPUID instruction), but
I'm not sure it matters.  I don't really see the point, myself.

    	     		    	  	 - Ted

^ permalink raw reply

* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Andy Lutomirski @ 2014-07-27 22:17 UTC (permalink / raw)
  To: Paolo Bonzini, linux-crypto, Henrique de Moraes Holschuh,
	linux-kernel@vger.kernel.org, James Morris, LSM List, Al Viro,
	Linux API, Julien Tinnes, Theodore Ts'o, Greg Kroah-Hartman,
	Paul Moore, David Drysdale, Eric W. Biederman, Kees Cook,
	Meredydd Luff, Christoph Hellwig
In-Reply-To: <20140727210617.GY6725@thunk.org>

On Jul 27, 2014 5:06 PM, "Theodore Ts'o" <tytso@mit.edu> wrote:
>
> On Fri, Jul 25, 2014 at 11:30:48AM -0700, Andy Lutomirski wrote:
> >
> > There is recent interest in having a way to turn generally-available
> > kernel features off.  Maybe we should add a good one so we can stop
> > bikeshedding and avoid proliferating dumb interfaces.
>
> I believe the seccomp infrastructure (which is already upstream)
> should be able to do most of what you want, at least with respect to
> features which are exposed via system calls (which was most of your
> list).

Seccomp can't really restrict lookups of non-self pids.  In fact, this
feature idea started out as a response to a patch adding a kind of
nasty seccomp feature to make it sort of possible.

I agree that that seccomp can turn off GRND_RANDOM, but how is it
supposed to do it in such a way that the filtered software will fall
back to something sensible?  -ENOSYS?  -EPERM?  Something else?

I think that -ENOSYS is clearly wrong, but standardizing this would be
nice.  Admittedly, adding something fancy like this for GRND_RANDOM
may not be appropriate.

--Andy

>
> It won't cover x86 specific things like restricting RDTSC or CPUID
> (and as far as I know you can't intercept the CPUID instruction), but
> I'm not sure it matters.  I don't really see the point, myself.
>
>                                          - Ted

^ permalink raw reply

* Re: General flags to turn things off (getrandom, pid lookup, etc)
From: Eric W. Biederman @ 2014-07-27 23:53 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Paolo Bonzini, linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	Henrique de Moraes Holschuh, linux-kernel@vger.kernel.org,
	James Morris, LSM List, Al Viro, Linux API, Julien Tinnes,
	Theodore Ts'o, Greg Kroah-Hartman, Paul Moore, David Drysdale,
	Kees Cook, Meredydd Luff, Christoph Hellwig
In-Reply-To: <CALCETrXfzOZEC3mOKkm3_GvitvkEu4kAa3LowRv-fjHnzrQnBg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:

> On Jul 27, 2014 5:06 PM, "Theodore Ts'o" <tytso-3s7WtUTddSA@public.gmane.org> wrote:
>>
>> On Fri, Jul 25, 2014 at 11:30:48AM -0700, Andy Lutomirski wrote:
>> >
>> > There is recent interest in having a way to turn generally-available
>> > kernel features off.  Maybe we should add a good one so we can stop
>> > bikeshedding and avoid proliferating dumb interfaces.
>>
>> I believe the seccomp infrastructure (which is already upstream)
>> should be able to do most of what you want, at least with respect to
>> features which are exposed via system calls (which was most of your
>> list).
>
> Seccomp can't really restrict lookups of non-self pids.  In fact, this
> feature idea started out as a response to a patch adding a kind of
> nasty seccomp feature to make it sort of possible.
>
> I agree that that seccomp can turn off GRND_RANDOM, but how is it
> supposed to do it in such a way that the filtered software will fall
> back to something sensible?  -ENOSYS?  -EPERM?  Something else?
>
> I think that -ENOSYS is clearly wrong, but standardizing this would be
> nice.  Admittedly, adding something fancy like this for GRND_RANDOM
> may not be appropriate.

Andy you seem to be arguing here for two system calls.
get_urandom() and get_random().

Where get_urandom only blocks if there is not enough starting entropy,
and get_random(GRND_RANDOM) blocks if there is currently not enough
entropy.

That would allow -ENOSYS to be the right return value and it would
simply things for everyone.

Eric

^ permalink raw reply

* Re: [RFC PATCHv2 00/11] Adding FreeBSD's Capsicum security framework
From: Paolo Bonzini @ 2014-07-28 12:30 UTC (permalink / raw)
  To: Eric W. Biederman, David Drysdale
  Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
	Andy Lutomirski, Paul Moore, Christoph Hellwig,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <871tt796i0.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

Il 26/07/2014 23:04, Eric W. Biederman ha scritto:
>> The most significant aspect of Capsicum is associating *rights* with
>> (some) file descriptors, so that the kernel only allows operations on an
>> FD if the rights permit it.  This allows userspace applications to
>> sandbox themselves by tightly constraining what's allowed with both
>> input and outputs; for example, tcpdump might restrict itself so it can
>> only read from the network FD, and only write to stdout.
>>
>> The kernel thus needs to police the rights checks for these file
>> descriptors (referred to as 'Capsicum capabilities', completely
>> different than POSIX.1e capabilities), and the best place to do this is
>> at the points where a file descriptor from userspace is converted to a
>> struct file * within the kernel.
>>
>>   [Policing the rights checks anywhere else, for example at the system
>>   call boundary, isn't a good idea because it opens up the possibility
>>   of time-of-check/time-of-use (TOCTOU) attacks [2] where FDs are
>>   changed (as openat/close/dup2 are allowed in capability mode) between
>>   the 'check' at syscall entry and the 'use' at fget() invocation.]
>>
>> However, this does lead to quite an invasive change to the kernel --
>> every invocation of fget() or similar functions (fdget(),
>> sockfd_lookup(), user_path_at(),...) needs to be annotated with the
>> rights associated with the specific operations that will be performed on
>> the struct file.  There are ~100 such invocations that need
>> annotation.
> 
> And it is silly.  Roughly you just need a locking version of
> fcntl(F_SETFL).
> 
> That is make the restriction in the struct file not in the fd to file
> lookup.

No, they have to be in the file descriptor.  The same file descriptor
can be dup'ed and passed with different capabilities to different processes.

Say you pass an eventfd to a process with SCM_RIGHTS, and you want to
only allow the process to write to it.

>> 4) New System Calls
>> -------------------
>>
>> To allow userspace applications to access the Capsicum capability
>> functionality, I'm proposing two new system calls: cap_rights_limit(2)
>> and cap_rights_get(2).  I guess these could potentially be implemented
>> elsewhere (e.g. as fcntl(2) operations?) but the changes seem
>> significant enough that new syscalls are warranted.
>>
>>   [FreeBSD 10.x actually includes six new syscalls for manipulating the
>>   rights associated with a Capsicum capability -- the capability rights
>>   can police that only specific fcntl(2) or ioctl(2) commands are
>>   allowed, and FreeBSD sets these with distinct syscalls.]
> 
> ioctls?  In a sandbox?  Ick.

KVM?  X11?  Both of them use loads of ioctls.  I'm less sure of the
benefit of picking which fcntls to allow.

Paolo

^ permalink raw reply

* Re: [RFC PATCHv2 00/11] Adding FreeBSD's Capsicum security framework
From: David Drysdale @ 2014-07-28 16:04 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: LSM List, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
	Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
	Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig,
	Linux API
In-Reply-To: <871tt796i0.fsf@x220.int.ebiederm.org>

On Sat, Jul 26, 2014 at 10:04 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> David Drysdale <drysdale@google.com> writes:
>
>> The last couple of versions of FreeBSD (9.x/10.x) have included the
>> Capsicum security framework [1], which allows security-aware
>> applications to sandbox themselves in a very fine-grained way.  For
>> example, OpenSSH now (>= 6.5) uses Capsicum in its FreeBSD version to
>> restrict sshd's credentials checking process, to reduce the chances of
>> credential leakage.
>>
>> It would be good to have equivalent functionality in Linux, so I've been
>> working on getting the Capsicum framework running in the kernel, and I'd
>> appreciate some feedback/opinions on the general approach.
>>
>> I'm attaching a corresponding draft patchset for reference, but
>> hopefully this cover email can cover the significant features to save
>> everyone having to look through the code details.  (It does mean this is
>> a long email though -- apologies for that.)
>>
>>
>> 1) Capsicum Capabilities
>> ------------------------
>>
>> The most significant aspect of Capsicum is associating *rights* with
>> (some) file descriptors, so that the kernel only allows operations on an
>> FD if the rights permit it.  This allows userspace applications to
>> sandbox themselves by tightly constraining what's allowed with both
>> input and outputs; for example, tcpdump might restrict itself so it can
>> only read from the network FD, and only write to stdout.
>>
>> The kernel thus needs to police the rights checks for these file
>> descriptors (referred to as 'Capsicum capabilities', completely
>> different than POSIX.1e capabilities), and the best place to do this is
>> at the points where a file descriptor from userspace is converted to a
>> struct file * within the kernel.
>>
>>   [Policing the rights checks anywhere else, for example at the system
>>   call boundary, isn't a good idea because it opens up the possibility
>>   of time-of-check/time-of-use (TOCTOU) attacks [2] where FDs are
>>   changed (as openat/close/dup2 are allowed in capability mode) between
>>   the 'check' at syscall entry and the 'use' at fget() invocation.]
>>
>> However, this does lead to quite an invasive change to the kernel --
>> every invocation of fget() or similar functions (fdget(),
>> sockfd_lookup(), user_path_at(),...) needs to be annotated with the
>> rights associated with the specific operations that will be performed on
>> the struct file.  There are ~100 such invocations that need
>> annotation.
>
> And it is silly.  Roughly you just need a locking version of
> fcntl(F_SETFL).
>
> That is make the restriction in the struct file not in the fd to file
> lookup.

The file status flags are far too coarse -- for example, O_RDONLY
doesn't prevent fchmod(2).

Also, because they're associated with the struct file there is no way
to pass a file descriptor with only a subset of rights across a UNIX
socket (how do I send a read-only FD corresponding to my
O_RDWR file?).

> Files in unix have been capabilities for more than 20 years.  That is
> what file descriptor passing in unix domain sockets are all about.  We
> don't need an additional ``capability rights'' layer on top of file
> descriptors.

True, the ability to pass FDs across UNIX sockets is one of the
key things that makes them analogous to object-capabilities and
suitable as the substratum for Capsicum.  But the coarseness of the
existing rights, and the lack of coherent policing of those rights,
means that an (opt-in) extension like Capsicum is useful.

> In fact internal to linux with FMODE_READ and friends we already have
> restrictions on which methods are allowed on linux file descriptors.
> So this whole entire abstraction layer you are adding seems just plain
> broken.
>
> Going farther one huge thing your work and the capsicum work in general
> is missing is an implementation of revoke.  With a little care a good
> implementation of bits reporting and controlling which methods are
> available on which file descriptors should be a good start on
> a revoke implementation for linux as well.

Yeah, revocation is interesting.  There was been some discussion of
it for Capsicum a while ago:
https://lists.cam.ac.uk/pipermail/cl-capsicum-discuss/2011-January/msg00002.html
but I don't think a firm conclusion was reached.  As I understand it,
there's also a theoretical argument that revocation can be constructed
on top of the capability system, by handing out capabilities to proxy
objects that can then change their behaviour so they no longer pass
operations through.

>> 2) Capsicum Capabilities Data Structure
>> ---------------------------------------
>>
>> Internally, the rights associated with a Capsicum capability FD are
>> stored in a special struct file wrapper.  For a normal file, the rights
>> check inside fget() falls through, but for a capability wrapper the
>> rights in the wrapper are checked and (if capable) the underlying
>> wrapped struct file is returned.
>>
>>   [This is approximately the implementation that was present in FreeBSD
>>   9.x.  For FreeBSD 10.x, the wrapper file was removed and the rights
>>   associated with a file descriptor are now stored in the fdtable. As
>>   that impacts memory use for all processes, whether Capsicum users or
>>   not, I've stuck with the FreeBSD 9.x approach.]
>
> I have already mentioned that this is an insane choice of semantics
> right?  Adding an extra layer on top of a data structure that is a
> perfectly good restrictor of rights.
>
> If you can't add the restriction on struct file itself I would argue
> that the semantics of capsicum are fundamentally broken.
>
> I can not imagine why in the world you would want and extra layer of
> indirection, complication, and maintenance.

See above.

>> 3) Allowing Capability Mode
>> ---------------------------
>>
>> Capsicum also includes 'capability mode', which locks down the available
>> syscalls so the rights restrictions can't just be bypassed by opening
>> new file descriptors.  More precisely, capability mode prevents access
>> to syscalls that access global namespaces, such as the filesystem or the
>> IP:port space.
>>
>> The existing seccomp-bpf functionality of the kernel is a good mechanism
>> for implementing capability mode, but there are a few additional details
>> that also need to be addressed.
>>
>>  a) The capability mode filter program needs to apply process-wide, not
>>     just to the current thread.
>>
>>  b) In capability mode, new files can still be opened with openat(2) but
>>     only if they are beneath an existing directory file descriptor.
>
> Which raises the question is that worth it?

>From a practical point of view, still allowing (directory-relative) filesystem
access means it's much easier for an application to adapt to Capsicum.

A simple example is something like unzip/tar -xf, where locking it down
is conceptually as simple as:
 - apply a CAP_READ restriction to the FD for the input file
 - apply a CAP_WRITE+CAP_LOOKUP restriction to the DFD for the
   output directory
 - enter capability mode.
Then the worst a malicious input file can do is to write files under the
output directory -- which it could do anyway.

Similarly, migrating an application that writes temporary files out to
some tempdir is much more straightforward if openat(dfd,...) is still
allowed.

>>  c) In capability mode it should still be possible for a process to send
>>     signals to itself with kill(2)/tgkill(2).
>
> Again is it worth it?
>
> I would think you would want capability mode to default to the minium
> set of system calls you could get away with (to keep kernel code
> auditing to a minium) and only add things if the performance gain of
> using the syscall exceeds the pain.

The set of syscalls allowed in capability mode is based more on the
principle of restricting access to global namespaces, to prevent (in
particular) the confused deputy problem.  Allowing kill(self) is a pragmatic
compromise to make it easier to migrate existing applications to use
Capsicum.

> If you look at a kernel like sel4 it succeeds in with an object
> capability model with many fewer system calls than you are proposing
> to export.  Roughly just read, write and close.
>
> Consider the fact if you really want a kernel layer you can completely
> trust and rely on someone needs to write a formal proof of that layer.
> Short of that someone certainly needs to audit the kernel code very
> closely so simplicity of semantics and simplicity of implementation are
> very important.

seL4 may have a much simpler (and more formally-correct) model, but
the aim of Capsicum is to get some of the benefits of that well-analysed
model without the need for a (massive) migration effort -- and in a way
that allows co-existence of migrated and unmigrated code.

>> This v2 patchset copes with these as follows:
>>
>>  a) Kees Cook's incoming seccomp(2) patchset covers thread
>>     synchronization of filters.
>>
>>  b) A new prctl(PR_SET_OPENAT_BENEATH) operation implicitly sets the
>>     O_BENEATH flag (see below) for all file-open operations for all
>>     threads of the current process, by adding a new openat_beneath
>>     flag in task_struct.
>>
>>  c) An extension to the seccomp_data structure that includes the current
>>     task's tid and tgid values allows for BPF programs that check a
>>     kill(2)/tgkill(2) argument against the current thread, in a manner
>>     that is robust against fork(2)/clone(2).
>>
>> The combination of these features with the existing seccomp-bpf
>> functionality gives the tools needed to implement capability mode.
>>
>>
>> 4) New System Calls
>> -------------------
>>
>> To allow userspace applications to access the Capsicum capability
>> functionality, I'm proposing two new system calls: cap_rights_limit(2)
>> and cap_rights_get(2).  I guess these could potentially be implemented
>> elsewhere (e.g. as fcntl(2) operations?) but the changes seem
>> significant enough that new syscalls are warranted.
>>
>>   [FreeBSD 10.x actually includes six new syscalls for manipulating the
>>   rights associated with a Capsicum capability -- the capability rights
>>   can police that only specific fcntl(2) or ioctl(2) commands are
>>   allowed, and FreeBSD sets these with distinct syscalls.]
>
> ioctls?  In a sandbox?  Ick.
>
>> 5) New openat(2) O_BENEATH Flag
>> -------------------------------
>>
>> For Capsicum capabilities that are directory file descriptors, the
>> Capsicum framework only allows openat(cap_dfd, path, ...) operations to
>> work for files that are beneath the specified directory (and even that
>> only when the directory FD has the CAP_LOOKUP right), rejecting paths
>> that start with "/" or include "..".  The same restriction applies
>> process-wide for a process in capability mode.
>>
>> As this seemed like functionality that might be more generally useful,
>> I've implemented it independently as a new O_BENEATH flag for openat(2).
>> The Capsicum code then always triggers the use of that flag when the dfd
>> is a Capsicum capability, or when the prctl(2) command described above
>> is in play.
>>
>>   [FreeBSD has the openat(2) relative-only behaviour for capability DFDs
>>   and processes in capability mode, but does not include the O_BENEATH
>>   flag.]
>
> If you are going to allow open I would think the simple solution here is
> to just create a mount namespace that only has available the files you
> would like to export to the process, and force all of the file
> descriptors into that mount namespace.
>
> Eric

I think that's a lot harder for application writers to do -- they would need
to have CAP_SYS_ADMIN to set up the namespace & mounts before
dropping privileges.  And the net result would again be much less
fine-grained (e.g. for the unzip example above, the specific capability
rights prevent reading of any files that already exist in the output
directory).

^ permalink raw reply

* Re: [RFC PATCHv2 00/11] Adding FreeBSD's Capsicum security framework
From: Eric W. Biederman @ 2014-07-28 21:13 UTC (permalink / raw)
  To: David Drysdale
  Cc: LSM List, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
	Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
	Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig,
	Linux API
In-Reply-To: <CAHse=S_7Jpb9bTXHgU8q8_UZ=ce2gwdq8C15a+jbfO_pHmV_iA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> writes:

> On Sat, Jul 26, 2014 at 10:04 PM, Eric W. Biederman
> <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>> David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> writes:

>>> 1) Capsicum Capabilities
>>> ------------------------
>>>
>>> The most significant aspect of Capsicum is associating *rights* with
>>> (some) file descriptors, so that the kernel only allows operations on an
>>> FD if the rights permit it.  This allows userspace applications to
>>> sandbox themselves by tightly constraining what's allowed with both
>>> input and outputs; for example, tcpdump might restrict itself so it can
>>> only read from the network FD, and only write to stdout.
>>>
>>> The kernel thus needs to police the rights checks for these file
>>> descriptors (referred to as 'Capsicum capabilities', completely
>>> different than POSIX.1e capabilities), and the best place to do this is
>>> at the points where a file descriptor from userspace is converted to a
>>> struct file * within the kernel.
>>>
>>>   [Policing the rights checks anywhere else, for example at the system
>>>   call boundary, isn't a good idea because it opens up the possibility
>>>   of time-of-check/time-of-use (TOCTOU) attacks [2] where FDs are
>>>   changed (as openat/close/dup2 are allowed in capability mode) between
>>>   the 'check' at syscall entry and the 'use' at fget() invocation.]
>>>
>>> However, this does lead to quite an invasive change to the kernel --
>>> every invocation of fget() or similar functions (fdget(),
>>> sockfd_lookup(), user_path_at(),...) needs to be annotated with the
>>> rights associated with the specific operations that will be performed on
>>> the struct file.  There are ~100 such invocations that need
>>> annotation.
>>
>> And it is silly.  Roughly you just need a locking version of
>> fcntl(F_SETFL).
>>
>> That is make the restriction in the struct file not in the fd to file
>> lookup.
>
> The file status flags are far too coarse -- for example, O_RDONLY
> doesn't prevent fchmod(2).
>
> Also, because they're associated with the struct file there is no way
> to pass a file descriptor with only a subset of rights across a UNIX
> socket (how do I send a read-only FD corresponding to my
> O_RDWR file?).

You can't.  Unix domain sockets pass struct file references.
That is passing a file descriptor with a unix domain sockets is the
equivlanet of dup().

You absolutely must make your struct file read-only before passing it,
otherwise the the receiver will have a read-write instance.

This notion that a shared structure should have different semantics
depending on who is looking at it, sounds like a maintenance nightmare
to me. 

>> Files in unix have been capabilities for more than 20 years.  That is
>> what file descriptor passing in unix domain sockets are all about.  We
>> don't need an additional ``capability rights'' layer on top of file
>> descriptors.
>
> True, the ability to pass FDs across UNIX sockets is one of the
> key things that makes them analogous to object-capabilities and
> suitable as the substratum for Capsicum.  But the coarseness of the
> existing rights, and the lack of coherent policing of those rights,
> means that an (opt-in) extension like Capsicum is useful.

Finer grained restrictions are perfectly sensible.

I don't see how it make sense to place those restrictions in
struct fdtable instead of in struct file.

I see two sensible implementations:
- Add a seccomp bpf filter to struct file.
- Add permission bits to struct file.

The bpf filter would allow for a simple code extension that would
allow any arbitrary policy to be applied.

Adding permisison bits to struct file for gating access to
file_operations and inode_operations would result in the fastest
possible implementation and something very easy to audit and
understand.  But there like ioctl or read/write on IB control files
where finer grained permissions might be desirable.

For a lot of operations we alread have bits like FMODE_READ and
FMODE_CAN_READ on struct file for reasons such as performance
so that only a single cache line needs to be touched.

It might be that it would be worth having both.  Something cheap and
genrally accessible and something fast.

>> Going farther one huge thing your work and the capsicum work in general
>> is missing is an implementation of revoke.  With a little care a good
>> implementation of bits reporting and controlling which methods are
>> available on which file descriptors should be a good start on
>> a revoke implementation for linux as well.
>
> Yeah, revocation is interesting.  There was been some discussion of
> it for Capsicum a while ago:
> https://lists.cam.ac.uk/pipermail/cl-capsicum-discuss/2011-January/msg00002.html
> but I don't think a firm conclusion was reached.  As I understand it,
> there's also a theoretical argument that revocation can be constructed
> on top of the capability system, by handing out capabilities to proxy
> objects that can then change their behaviour so they no longer pass
> operations through.

I believe the capsicum approach is so far from allowing general proxy
objects that, that is not a compelling case.

>>> 3) Allowing Capability Mode
>>> ---------------------------
>>>
>>> Capsicum also includes 'capability mode', which locks down the available
>>> syscalls so the rights restrictions can't just be bypassed by opening
>>> new file descriptors.  More precisely, capability mode prevents access
>>> to syscalls that access global namespaces, such as the filesystem or the
>>> IP:port space.
>>>
>>> The existing seccomp-bpf functionality of the kernel is a good mechanism
>>> for implementing capability mode, but there are a few additional details
>>> that also need to be addressed.
>>>
>>>  a) The capability mode filter program needs to apply process-wide, not
>>>     just to the current thread.
>>>
>>>  b) In capability mode, new files can still be opened with openat(2) but
>>>     only if they are beneath an existing directory file descriptor.
>>
>> Which raises the question is that worth it?
>
> From a practical point of view, still allowing (directory-relative) filesystem
> access means it's much easier for an application to adapt to Capsicum.
>
> A simple example is something like unzip/tar -xf, where locking it down
> is conceptually as simple as:
>  - apply a CAP_READ restriction to the FD for the input file
>  - apply a CAP_WRITE+CAP_LOOKUP restriction to the DFD for the
>    output directory
>  - enter capability mode.
> Then the worst a malicious input file can do is to write files under the
> output directory -- which it could do anyway.
>
> Similarly, migrating an application that writes temporary files out to
> some tempdir is much more straightforward if openat(dfd,...) is still
> allowed.


>>>  c) In capability mode it should still be possible for a process to send
>>>     signals to itself with kill(2)/tgkill(2).


> seL4 may have a much simpler (and more formally-correct) model, but
> the aim of Capsicum is to get some of the benefits of that well-analysed
> model without the need for a (massive) migration effort -- and in a way
> that allows co-existence of migrated and unmigrated code.

Alright. About the same target as seccomp then.  Aiming at something
that is simple to adopt sounds like a reasonable goal.

>>> 5) New openat(2) O_BENEATH Flag
>>> -------------------------------
>>>
>>> For Capsicum capabilities that are directory file descriptors, the
>>> Capsicum framework only allows openat(cap_dfd, path, ...) operations to
>>> work for files that are beneath the specified directory (and even that
>>> only when the directory FD has the CAP_LOOKUP right), rejecting paths
>>> that start with "/" or include "..".  The same restriction applies
>>> process-wide for a process in capability mode.
>>>
>>> As this seemed like functionality that might be more generally useful,
>>> I've implemented it independently as a new O_BENEATH flag for openat(2).
>>> The Capsicum code then always triggers the use of that flag when the dfd
>>> is a Capsicum capability, or when the prctl(2) command described above
>>> is in play.
>>>
>>>   [FreeBSD has the openat(2) relative-only behaviour for capability DFDs
>>>   and processes in capability mode, but does not include the O_BENEATH
>>>   flag.]
>>
>> If you are going to allow open I would think the simple solution here is
>> to just create a mount namespace that only has available the files you
>> would like to export to the process, and force all of the file
>> descriptors into that mount namespace.

> I think that's a lot harder for application writers to do -- they would need
> to have CAP_SYS_ADMIN to set up the namespace & mounts before
> dropping privileges.  

They only need CAP_SYS_ADMIN in their local user namespace, which anyone
is allowed to create.

> And the net result would again be much less
> fine-grained (e.g. for the unzip example above, the specific capability
> rights prevent reading of any files that already exist in the output
> directory).

Nope.  What you can implement today if you want fine grained limitations
like this is to create a mount namespace with exactly the subdirectory
tree you want to allow access to and to return a file descriptor that
points into that mount namespace.  (When complete the only user of that
mount namespace would be your file descriptor).

In fact that solution is sufficiently performant and simple that even
if you came up with a better user space interface for it that is how we
would want to implement it.

And frankly that already exists today so I think a fairly large burden
of proof needs to be met to suggest that we need to add additional
functionality to the kernel.

Furthermore I think it makes sense for application authors to use
as otherwise they will have to wait a year or so for the debates to be
finished and your new functionality to be merged.

Additionally unless there is a process wide restriction to relative
paths I can trivially escape your relative path implementation by simply
doing open(.) and getting a struct file without any of those
restrictions.

Eric

^ permalink raw reply

* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Eric W. Biederman @ 2014-07-28 21:18 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Kees Cook, Julien Tinnes, David Drysdale, Al Viro, Paolo Bonzini,
	LSM List, Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
	Meredydd Luff, Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrVChObsQpL6dt-ByiCjbPrtpXAXQgy_apBY-OpGQHaPjg@mail.gmail.com>

Andy Lutomirski <luto@amacapital.net> writes:

> [cc: Eric Biederman]
>

> Can we do one better and add a flag to prevent any non-self pid
> lookups?  This might actually be easy on top of the pid namespace work
> (e.g. we could change the way that find_task_by_vpid works).
>
> It's far from just being signals.  There's access_process_vm, ptrace,
> all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
> is ridiculous), and probably some others that I've forgotten about or
> never noticed in the first place.

So here is the practical question.

Are these processes that only can send signals to their thread group
allowed to call fork()?


If fork is allowed and all pid lookups are restricted to their own
thread group that wait, waitpid, and all of the rest of the wait family
will never return the pids of their children, and zombies will
accumulate.  Aka the semantics are fundamentally broken.

If fork is not allowed pid namespaces already solve this problem.

Eric

^ permalink raw reply

* Re: [RFC PATCH 1/1] ethtool: adding support for multiple slave port configuration
From: Andy Gospodarek @ 2014-07-29  1:03 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Mugunthan V N, netdev, davem, linux-api, linux-kernel
In-Reply-To: <1406429221.29010.151.camel@deadeye.wl.decadent.org.uk>

On Sun, Jul 27, 2014 at 03:47:01AM +0100, Ben Hutchings wrote:
> On Fri, 2014-07-25 at 17:58 +0530, Mugunthan V N wrote:
> > Some Ethernet Swtich controllers like CPSW in AM335x, TI814x, DRA7x and
> > AM43xx SoCs, Network Coprocessor in AM5K2E0x, Realtek Switch controllers
> > etc has to capability of conneting multiple networks using L2 switching
> > and has multiple phys. With the existing code, ethtool can communicate
> > only to one phy.
> > 
> > To enable user to communicate multiple phy connected to single Ethernet
> > Switch controller, intoducing a optional new parameter in Ethtool interface
> > to pass which slave to set/get the phy configuration.
> 
> There was some discussion about configuration APIs for hardware/firmware
> bridges earlier this year and I thought there was a consensus for
> assigning a network device to each port.  This would remove the need to
> identify ports within a device.  But I may have misremembered.

Using a netdev per port was more or less the concensus for how to handle
the management of the individual ports connected to a hardware/firmware
switch.  Various suggestions were made to add a new struct (or not add
one) to at least link the netdevs used for each port to capture and
describe the hardware/firmware switch.

If individual netdevs are created (even if their functionality is
extremely limited) you could then use those devices with the base switch
driver that exists now, correct?  It seems like this must be an option
as this patch is simply a way to export this data/configuration to
userspace via ethtool.

> 
> > Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> > ---
> >  include/uapi/linux/ethtool.h | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > index 96ade34..3011427 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -60,6 +60,9 @@
> >   *	and other link features that the link partner advertised
> >   *	through autonegotiation; 0 if unknown or not applicable.
> >   *	Read-only.
> > + * @slave_port: Specify which slave port to be used to set/get
> > + *	parmeters, for example which slave port phy to be used for
> > + *	set/get phy capabilities
> 
> The difficulty with assigning the reserved fields in struct ethtool_cmd
> is that nothing has ever checked that they are set to 0.  So if we were
> to assign this field and support it in ethtool, someone might run it on
> an older kernel version and all configuration changes will be made to
> port 0 rather than the one they specified.  I don't think it would be
> acceptable to tell users that 'oh, the port number option silently fails
> on older kernel versions'.
> 
> So at the very least you would also need to add some way for userland to
> find out whether the driver will check the value of this field.
> 
> Ben.
> 
> >   * The link speed in Mbps is split between @speed and @speed_hi.  Use
> >   * the ethtool_cmd_speed() and ethtool_cmd_speed_set() functions to
> > @@ -107,7 +110,8 @@ struct ethtool_cmd {
> >  	__u8	eth_tp_mdix;
> >  	__u8	eth_tp_mdix_ctrl;
> >  	__u32	lp_advertising;
> > -	__u32	reserved[2];
> > +	__u32	slave_port;
> > +	__u32	reserved;
> >  };
> >  
> >  static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
> 
> -- 
> Ben Hutchings
> The generation of random numbers is too important to be left to chance.
>                                                             - Robert Coveyou

^ permalink raw reply

* Re: [PATCH 0/5] RFC: CGroup Namespaces
From: Serge E. Hallyn @ 2014-07-29  4:51 UTC (permalink / raw)
  To: Aditya Kali
  Cc: Serge Hallyn, Linux API, Linux Containers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Andy Lutomirski, Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA,
	Ingo Molnar
In-Reply-To: <CAGr1F2GcAema-E2q6PFj=R0Z505iD7JshrMuMdfPTJ95wMiQMA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Quoting Aditya Kali (adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org):
> Thank you for your review. I have tried to respond to both your emails here.
> 
> On Thu, Jul 24, 2014 at 9:36 AM, Serge Hallyn <serge.hallyn-GeWIH/nMZzJhl2p70BpVqQ@public.gmane.orgm> wrote:
> > Quoting Aditya Kali (adityakali-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org):
> >> Background
> >>   Cgroups and Namespaces are used together to create “virtual”
> >>   containers that isolates the host environment from the processes
> >>   running in container. But since cgroups themselves are not
> >>   “virtualized”, the task is always able to see global cgroups view
> >>   through cgroupfs mount and via /proc/self/cgroup file.
> >>
> > Hi,
> >
> > A few questions/comments:
> >
> > 1. Based on this description, am I to understand that after doing a
> >    cgroupns unshare, 'mount -t cgroup cgroup /mnt' by default will
> >    still mount the global root cgroup?  Any plans on "changing" that?
> 
> This is suggested in the "Possible Extensions of CGROUPNS" section.
> More details below.
> 
> >    Will attempts to change settings of a cgroup which is not under
> >    our current ns be rejected?  (That should be easy to do given your
> >    patch 1/5).  Sorry if it's done in the set, I'm jumping around...
> >
> 
> Currently, only 'cgroup_attach_task', 'cgroup_mkdir' and
> 'cgroup_rmdir' of cgroups outside of cgroupns-root are prevented. The
> read/write of actual cgroup properties are not prevented. Usual
> permission checks continue to apply for those. I was hoping that
> should be enough, but see more comments towards the end.
> 
> > 2. What would be the reprecussions of allowing cgroupns unshare so
> >    long as you have ns_capable(CAP_SYS_ADMIN) to the user_ns which
> >    created your current ns cgroup?  It'd be a shame if that wasn't
> >    on the roadmap.
> >
> 
> Its certainly on the roadmap, just that some logistics were not clear
> at this time. As pointed out by Andy Lutomirski on [PATCH 5/5] of this
> series, if we allow cgroupns creation to ns_capable(CAP_SYS_ADMIN)
> processes, we may need some kind of explicit permission from the
> cgroup subsystem to allow this. One approach could be an explicit

So long as you do ns_capable(cgroup_ns->user_ns, CAP_SYS_ADMIN) I think
you're fine.

The only real problem I can think of with unsharing a cgroup_ns is that
you could lock a setuid-root application someplace it wasn't expecting.
The above check guarantees that you were privileged enough that you'd
better be trusted in this user namespace.

(Unless there is some possible interaction I'm overlooking)

> cgroup.may_unshare setting. Alternatively, the cgroup directory (which
> is going to become the cgroupns-root) ownership could also be used
> here. i.e., the process is ns_capable(CAP_SYS_ADMIN) && it owns the
> cgroup directory. There seems to be already a function that allows
> similar thing and might be sufficient:
> 
> /**
>  * capable_wrt_inode_uidgid - Check nsown_capable and uid and gid mapped
>  * @inode: The inode in question
>  * @cap: The capability in question
>  *
>  * Return true if the current task has the given capability targeted at
>  * its own user namespace and that the given inode's uid and gid are
>  * mapped into the current user namespace.
>  */
> bool capable_wrt_inode_uidgid(const struct inode *inode, int cap)
> 
> What do you think? We can enable this for non-init userns once this is
> decided on.

I don't think it's needed... (until you show how wrong I am above :)

> > 3. The un-namespaced view of /proc/self/cgroup from a sibling cgroupns
> >    makes me wonder whether it wouldn't be more appropriate to leave
> >    /proc/self/cgroup always un-filtered, and use /proc/self/nscgroup
> >    (or somesuch) to provide the namespaced view.  /proc/self/nscgroup
> >    would simply be empty (or say (invalid) or (unreachable)) from a
> >    sibling ns.  That will give criu and admin tools like lxc/docker all
> >    they need to do simple cgroup setup.
> >
> 
> It may work for lxc/docker and new applications that use the new
> interface. But its difficult to change numerous existing user
> applications and libraries that depend on /proc/self/cgroup. Moreover,
> even with the new interface, /proc/self/cgroup will continue to leak
> system level cgroup information. And fixing this leak is critical to
> make the container migratable.
> 
> Its easy to correctly handle the read of /proc/<pid>/cgroup from a
> sibling cgroupns. Instead of showing unfiltered view, we could just
> not show anything (same behavior when the cgroup hierarchy is not
> mounted). Will that be more acceptable? I can make that change in the
> next version of this series.

It'll be acceptable so long as setns(CLONE_NEWCGROUP) is supported.

> >>   $ cat /proc/self/cgroup
> >>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
> >>
> >>   This exposure of cgroup names to the processes running inside a
> >>   container results in some problems:
> >>   (1) The container names are typically host-container-management-agent
> >>       (systemd, docker/libcontainer, etc.) data and leaking its name (or
> >>       leaking the hierarchy) reveals too much information about the host
> >>       system.
> >>   (2) It makes the container migration across machines (CRIU) more
> >>       difficult as the container names need to be unique across the
> >>       machines in the migration domain.
> >>   (3) It makes it difficult to run container management tools (like
> >>       docker/libcontainer, lmctfy, etc.) within virtual containers
> >>       without adding dependency on some state/agent present outside the
> >>       container.
> >>
> >>   Note that the feature proposed here is completely different than the
> >>   “ns cgroup” feature which existed in the linux kernel until recently.
> >>   The ns cgroup also attempted to connect cgroups and namespaces by
> >>   creating a new cgroup every time a new namespace was created. It did
> >>   not solve any of the above mentioned problems and was later dropped
> >>   from the kernel.
> >>
> >> Introducing CGroup Namespaces
> >>   With unified cgroup hierarchy
> >>   (Documentation/cgroups/unified-hierarchy.txt), the containers can now
> >>   have a much more coherent cgroup view and its easy to associate a
> >>   container with a single cgroup. This also allows us to virtualize the
> >>   cgroup view for tasks inside the container.
> >>
> >>   The new CGroup Namespace allows a process to “unshare” its cgroup
> >>   hierarchy starting from the cgroup its currently in.
> >>   For Ex:
> >>   $ cat /proc/self/cgroup
> >>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
> >>   $ ls -l /proc/self/ns/cgroup
> >>   lrwxrwxrwx 1 root root 0 2014-07-15 10:37 /proc/self/ns/cgroup -> cgroup:[4026531835]
> >>   $ ~/unshare -c  # calls unshare(CLONE_NEWCGROUP) and exec’s /bin/bash
> >>   [ns]$ ls -l /proc/self/ns/cgroup
> >>   lrwxrwxrwx 1 root root 0 2014-07-15 10:35 /proc/self/ns/cgroup -> cgroup:[4026532183]
> >>   # From within new cgroupns, process sees that its in the root cgroup
> >>   [ns]$ cat /proc/self/cgroup
> >>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
> >>
> >>   # From global cgroupns:
> >>   $ cat /proc/<pid>/cgroup
> >>   0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1
> >>
> >>   The virtualization of /proc/self/cgroup file combined with restricting
> >>   the view of cgroup hierarchy by bind-mounting for the
> >>   $CGROUP_MOUNT/batchjobs/c_job_id1/ directory to
> >>   $CONTAINER_CHROOT/sys/fs/cgroup/) should provide a completely isolated
> >>   cgroup view inside the container.
> >>
> >>   In its current simplistic form, the cgroup namespaces provide
> >>   following behavior:
> >>
> >>   (1) The “root” cgroup for a cgroup namespace is the cgroup in which
> >>       the process calling unshare is running.
> >>       For ex. if a process in /batchjobs/c_job_id1 cgroup calls unshare,
> >>       cgroup /batchjobs/c_job_id1 becomes the cgroupns-root.
> >>       For the init_cgroup_ns, this is the real root (“/”) cgroup
> >>       (identified in code as cgrp_dfl_root.cgrp).
> >>
> >>   (2) The cgroupns-root cgroup does not change even if the namespace
> >>       creator process later moves to a different cgroup.
> >>       $ ~/unshare -c # unshare cgroupns in some cgroup
> >>       [ns]$ cat /proc/self/cgroup
> >>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/
> >>       [ns]$ mkdir sub_cgrp_1
> >>       [ns]$ echo 0 > sub_cgrp_1/cgroup.procs
> >>       [ns]$ cat /proc/self/cgroup
> >>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
> >>
> >>   (3) Each process gets its CGROUPNS specific view of
> >>       /proc/<pid>/cgroup.
> >>   (a) Processes running inside the cgroup namespace will be able to see
> >>       cgroup paths (in /proc/self/cgroup) only inside their root cgroup
> >>       [ns]$ sleep 100000 &  # From within unshared cgroupns
> >>       [1] 7353
> >>       [ns]$ echo 7353 > sub_cgrp_1/cgroup.procs
> >>       [ns]$ cat /proc/7353/cgroup
> >>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/sub_cgrp_1
> >>
> >>   (b) From global cgroupns, the real cgroup path will be visible:
> >>       $ cat /proc/7353/cgroup
> >>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
> >>
> >>   (c) From a sibling cgroupns, the real path will be visible:
> >>       [ns2]$ cat /proc/7353/cgroup
> >>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
> >>       (In correct container setup though, it should not be possible to
> >>        access PIDs in another container in the first place. This can be
> >>        detected changed if desired.)
> >>
> >>   (4) Processes inside a cgroupns are not allowed to move out of the
> >>       cgroupns-root. This is true even if a privileged process in global
> >>       cgroupns tries to move the process out of its cgroupns-root.
> >>
> >>       # From global cgroupns
> >>       $ cat /proc/7353/cgroup
> >>       0:cpuset,cpu,cpuacct,memory,devices,freezer,hugetlb:/batchjobs/c_job_id1/sub_cgrp_1
> >>       # cgroupns-root for 7353 is /batchjobs/c_job_id1
> >>       $ echo 7353 > batchjobs/c_job_id2/cgroup.procs
> >>       -bash: echo: write error: Operation not permitted
> >>
> >>   (5) setns() is not supported for cgroup namespace in the initial
> >>       version.
> >
> > This combined with the full-path reporting for peer ns cgroups could make
> > for fun antics when attaching to an existing container (since we'd have
> > to unshare into a new ns cgroup with the same roto as the container).
> > I understand you are implying this will be fixed soon though.
> >
> 
> I am thinking the setns() will be only allowed if
> target_cgrpns->cgroupns_root is_descendant_of
> current_cgrpns->cgroupns_root. i.e., you will only be setns to a
> cgroup namespace which is rooted deeper in hierarchy than your own (in
> addition to checking capable_wrt_inode_uidgid(target_cgrpns_inode)).

Certainly.

> In addition to this, we need to decide whether its OK for setns() to
> also change the cgroup of the task. Consider following example:
> 
> [A] ----> [B] ----> C
>     ----> D
> 
> [A] and [B] are cgroupns-roots. Now, if a task in Cgroup D (which is
> under cgroupns [A]) attempts to setns() to cgroupns [B], then its
> cgroup should change from /A/D to /A/B. I am concerned about the
> side-effects this might cause. Though otherwise, this is a very useful
> feature for containers. One could argue that this is similar to
> setns() to a mount-namespace which is pivot_root'd somewhere else (in
> which case, the attaching task's root "/" moves implicitly with
> setns).

This is what I'd expect.

> Alternatively, we could only allow setns() if
> target_cgrpns->cgroupns_root == current->cgroup . I.e., taking above
> example again, if process in Cgroup D wants to setns() to cgroupns
> [B], then it will first need to move to Cgroup B, and only then the
> setns() will succeed. This makes sure that there is no implicit cgroup
> move.

I'm ok with the restriction if it makes the patchset easier for you -
i.e. you not having to man-handle me into another cgroup.  Though I
wouldn't expect the locking for that to be an obstacle...

> WDYT? I haven't prototyped this yet, but will send out a patch after
> this series is accepted.

Either one is fine with me.

> >>   (6) When some thread from a multi-threaded process unshares its
> >>       cgroup-namespace, the new cgroupns gets applied to the entire
> >>       process (all the threads). This should be OK since
> >>       unified-hierarchy only allows process-level containerization. So
> >>       all the threads in the process will have the same cgroup. And both
> >>       - changing cgroups and unsharing namespaces - are protected under
> >>       threadgroup_lock(task).
> >>
> >>   (7) The cgroup namespace is alive as long as there is atleast 1
> >>       process inside it. When the last process exits, the cgroup
> >>       namespace is destroyed. The cgroupns-root and the actual cgroups
> >>       remain though.
> >>
> >> Implementation
> >>   The current patch-set is based on top of Tejun's cgroup tree (for-next
> >>   branch). Its fairly non-intrusive and provides above mentioned
> >>   features.
> >>
> >> Possible extensions of CGROUPNS:
> >>   (1) The Documentation/cgroups/unified-hierarchy.txt mentions use of
> >>       capabilities to restrict cgroups to administrative users. CGroup
> >>       namespaces could be of help here. With cgroup namespaces, it might
> >>       be possible to delegate administration of sub-cgroups under a
> >>       cgroupns-root to the cgroupns owner.
> >
> > That would be nice.
> >
> >>   (2) Provide a cgroupns specific cgroupfs mount. i.e., the following
> >>       command when ran from inside a cgroupns should only mount the
> >>       hierarchy from cgroupns-root cgroup:
> >>       $ mount -t cgroup cgroup <cgroup-mountpoint>
> >>       # -o __DEVEL__sane_behavior should be implicit
> >>
> >>       This is similar to how procfs can be mounted for every PIDNS. This
> >>       may have some usecases.
> >
> > Sorry - I see this answers the first part of a question in my previous email.
> > However, the question of whether changes to limits in cgroups which are not
> > under our cgroup-ns-root are allowed.
> >
> > Admittedly the current case with cgmanager is the same - in that it depends
> > on proper setup of the container - but cgmanager is geared to recommend
> > not mounting the cgroups in the container at all (and we can reject such
> > mounts in the contaienr altogether with no loss in functionality) whereas
> > you are here encouraging such mounts.  Which is fine - so long as you then
> > fully address the potential issues.
> 
> It will be nice to have this, but frankly, it may add a bit of
> complexity in the cgroup/kernfs code (I will have to prototype and
> see). Also same behavior can be obtained simply by bind-mounting
> cgroupns-root inside the container. So I am currently inclining
> towards rejecting such mounts in favor of simplicity.

Not having to track what to bind-mount where is a very nice
simplification though.  In lxc with cgmanager, we are now able to always
simply bind-mount /sys/fs/cgroup/cgmanager from the host into the
container.  Nothing more needed for the container to be able to manage
its own cgroup and start its own containers.  Likewise, if mount -t
cgroup were filtered to cgroupns, then lxc could simply not mount
anything into the container at all.  If it mount -t cgroup is not
filtered wrt cgroupns, then we'd have to go back to, at container start,
finding the mountpoint for every subsystem, calculating the container's
cgroup there, and bind-mounting them into the container.

> Regarding disallowing writes to cgroup files outside of your
> cgroupns-root, I think it should possible implement it easily. I will
> include it in the next revision of this series.

Great - thanks.

-serge

^ permalink raw reply

* Re: [RFC PATCH 0/5] xgetdents system call
From: Michael Kerrisk @ 2014-07-29  8:18 UTC (permalink / raw)
  To: Abhi Das
  Cc: Linux Kernel, Linux-Fsdevel, cluster-devel-H+wXaHxf7aLQT0dZR+AlfA,
	Linux API
In-Reply-To: <1406309888-10749-1-git-send-email-adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Fri, Jul 25, 2014 at 7:38 PM, Abhi Das <adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> This system call takes 5 arguments:
> fd    - file descriptor of the directory
> flags - flags used by xstat
> mask  - field mask used to request stat/xattr info
> buf   - user buf to return collected info
> count - size of user buffer in bytes
>
> xgetdents() should read the directory entries, collect stat
> and xattr information for each entry as requested and return
> all the data back to the user in a container structure
> (linux_xdirent) as part of the supplied user buffer.

Hello Abhi,

As per Documentation/SubmitChecklist, please CC linux-api on patches
that change the kerne-user-space API/ABI. (See
https://www.kernel.org/doc/man-pages/linux-api-ml.html for more
details.)

Cheers,

Michael



> Abhi Das (5):
>   fs: xstat system call VFS bits
>   fs: Add xgetdents system call and xreaddir file operation
>   gfs2: Add a dynamic buffer backed by a vector of pages
>   gfs2: Add sort functionality with extra parameter
>   gfs2: Add xreaddir file operation and supporting functions
>
>  arch/x86/syscalls/syscall_32.tbl |    3 +
>  arch/x86/syscalls/syscall_64.tbl |    3 +
>  fs/gfs2/Makefile                 |    3 +-
>  fs/gfs2/dir.c                    |   80 +--
>  fs/gfs2/dir.h                    |   13 +-
>  fs/gfs2/export.c                 |    2 +-
>  fs/gfs2/file.c                   |   17 +-
>  fs/gfs2/incore.h                 |    6 +
>  fs/gfs2/inode.c                  |    3 +-
>  fs/gfs2/inode.h                  |    5 +
>  fs/gfs2/ops_fstype.c             |    4 +
>  fs/gfs2/sys.c                    |   26 +-
>  fs/gfs2/util.c                   |  363 ++++++++++++++
>  fs/gfs2/util.h                   |   47 ++
>  fs/gfs2/xattr.c                  |   27 +-
>  fs/gfs2/xattr.h                  |   23 +
>  fs/gfs2/xreaddir.c               | 1024 ++++++++++++++++++++++++++++++++++++++
>  fs/gfs2/xreaddir.h               |   84 ++++
>  fs/readdir.c                     |   42 ++
>  fs/stat.c                        |  340 ++++++++++++-
>  include/linux/fs.h               |    5 +
>  include/linux/stat.h             |   16 +-
>  include/linux/syscalls.h         |    5 +
>  include/uapi/linux/fcntl.h       |    1 +
>  include/uapi/linux/stat.h        |  143 ++++++
>  25 files changed, 2196 insertions(+), 89 deletions(-)
>  create mode 100644 fs/gfs2/xreaddir.c
>  create mode 100644 fs/gfs2/xreaddir.h
>
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

^ permalink raw reply

* Re: [RFC PATCH 0/2] dirreadahead system call
From: Michael Kerrisk @ 2014-07-29  8:19 UTC (permalink / raw)
  To: Abhi Das; +Cc: Linux Kernel, Linux-Fsdevel, cluster-devel, Linux API
In-Reply-To: <1406309851-10628-1-git-send-email-adas@redhat.com>

On Fri, Jul 25, 2014 at 7:37 PM, Abhi Das <adas@redhat.com> wrote:
> This system call takes 3 arguments:
> fd      - file descriptor of the directory being readahead
> *offset - offset in dir from which to resume. This is updated
>           as we move along in the directory
> count   - The max number of entries to readahead
>
> The syscall is supposed to read upto 'count' entries starting at
> '*offset' and cache the inodes corresponding to those entries. It
> returns a negative error code or a positive number indicating
> the number of inodes it has issued readaheads for. It also
> updates the '*offset' value so that repeated calls to dirreadahead
> can resume at the right location. Returns 0 when there are no more
> entries left.

Hello Abhi,

As per Documentation/SubmitChecklist, please CC linux-api on patches
that change the kerne-user-space API/ABI. (See
https://www.kernel.org/doc/man-pages/linux-api-ml.html for more
details.)

Cheers,

Michael


> Abhi Das (2):
>   fs: Add dirreadahead syscall and VFS hooks
>   gfs2: GFS2's implementation of the dir_readahead file operation
>
>  arch/x86/syscalls/syscall_32.tbl |   1 +
>  arch/x86/syscalls/syscall_64.tbl |   1 +
>  fs/gfs2/Makefile                 |   3 +-
>  fs/gfs2/dir.c                    |  49 ++++++---
>  fs/gfs2/dir.h                    |  15 +++
>  fs/gfs2/dir_readahead.c          | 209 +++++++++++++++++++++++++++++++++++++++
>  fs/gfs2/file.c                   |   2 +
>  fs/gfs2/main.c                   |  10 +-
>  fs/gfs2/super.c                  |   1 +
>  fs/readdir.c                     |  49 +++++++++
>  include/linux/fs.h               |   3 +
>  11 files changed, 328 insertions(+), 15 deletions(-)
>  create mode 100644 fs/gfs2/dir_readahead.c
>
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

^ permalink raw reply

* Re: [RFC PATCH 2/5] fs: Add xgetdents system call and xreaddir file operation
From: Michael Kerrisk @ 2014-07-29  8:20 UTC (permalink / raw)
  To: Abhi Das
  Cc: Linux Kernel, Linux-Fsdevel, cluster-devel-H+wXaHxf7aLQT0dZR+AlfA,
	Linux API
In-Reply-To: <1406309888-10749-3-git-send-email-adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[CC += linux-api]

On Fri, Jul 25, 2014 at 7:38 PM, Abhi Das <adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Also add linux_xdirent structure that will be the container for
> dirent, stat and xattr info.
>
> Signed-off-by: Abhi Das <adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/x86/syscalls/syscall_32.tbl |  1 +
>  arch/x86/syscalls/syscall_64.tbl |  1 +
>  fs/readdir.c                     | 42 ++++++++++++++++++++++++++++++++++++++++
>  fs/stat.c                        |  4 +++-
>  include/linux/fs.h               |  1 +
>  include/linux/stat.h             |  2 ++
>  include/uapi/linux/stat.h        | 33 +++++++++++++++++++++++++++++++
>  7 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
> index 6d6ca37..35723e3 100644
> --- a/arch/x86/syscalls/syscall_32.tbl
> +++ b/arch/x86/syscalls/syscall_32.tbl
> @@ -362,3 +362,4 @@
>  353    i386    renameat2               sys_renameat2
>  354    i386    xstat                   sys_xstat
>  355    i386    fxstat                  sys_fxstat
> +356    i386    xgetdents               sys_xgetdents
> diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
> index 1308ee3..566aab1 100644
> --- a/arch/x86/syscalls/syscall_64.tbl
> +++ b/arch/x86/syscalls/syscall_64.tbl
> @@ -325,6 +325,7 @@
>  316    common  renameat2               sys_renameat2
>  317    common  xstat                   sys_xstat
>  318    common  fxstat                  sys_fxstat
> +319    common  xgetdents               sys_xgetdents
>
>  #
>  # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 33fd922..d676088 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -224,6 +224,48 @@ SYSCALL_DEFINE3(getdents, unsigned int, fd,
>         return error;
>  }
>
> +SYSCALL_DEFINE5(xgetdents, unsigned int, fd, unsigned, flags, unsigned int, mask,
> +               void __user *, buf, unsigned int, count)
> +{
> +       struct fd f;
> +       struct inode *inode;
> +       int error = -ENOTDIR;
> +
> +       if (!count)
> +               return -EINVAL;
> +
> +       if (!access_ok(VERIFY_WRITE, buf, count))
> +               return -EFAULT;
> +
> +       f = fdget(fd);
> +       if (!f.file)
> +               return -EBADF;
> +
> +       inode = f.file->f_path.dentry->d_inode;
> +
> +       error = -ENOTSUPP;
> +       if (!f.file->f_op || !f.file->f_op->xreaddir)
> +               goto out;
> +
> +       error = security_file_permission(f.file, MAY_READ);
> +       if (error)
> +               goto out;
> +
> +       error = mutex_lock_killable(&inode->i_mutex);
> +       if (error)
> +               goto out;
> +
> +       error = -ENOENT;
> +       if (!IS_DEADDIR(inode)) {
> +               error = f.file->f_op->xreaddir(f.file, flags, mask, buf, count);
> +               file_accessed(f.file);
> +       }
> +       mutex_unlock(&inode->i_mutex);
> +out:
> +       fdput(f);
> +       return error;
> +}
> +
>  struct getdents_callback64 {
>         struct dir_context ctx;
>         struct linux_dirent64 __user * current_dir;
> diff --git a/fs/stat.c b/fs/stat.c
> index 1fd0b3e..db45f8b 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -651,7 +651,7 @@ static int xstat_get_params(unsigned int mask, struct xstat __user *buffer,
>   * Otherwise we copy the extended stats to userspace and return the amount of
>   * data written into the buffer (or -EFAULT).
>   */
> -static long xstat_set_result(struct kstat *stat, struct xstat __user *buffer)
> +long xstat_set_result(struct kstat *stat, struct xstat __user *buffer)
>  {
>         u32 mask = stat->result_mask, gran = stat->tv_granularity;
>
> @@ -701,6 +701,8 @@ static long xstat_set_result(struct kstat *stat, struct xstat __user *buffer)
>         return 0;
>  }
>
> +EXPORT_SYMBOL(xstat_set_result);
> +
>  /*
>   * System call to get extended stats by path
>   */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index b91f235..79c7d39 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1464,6 +1464,7 @@ struct file_operations {
>         ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
>         ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
>         int (*iterate) (struct file *, struct dir_context *);
> +       size_t (*xreaddir) (struct file *, unsigned int, unsigned int, void __user *, size_t);
>         unsigned int (*poll) (struct file *, struct poll_table_struct *);
>         long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
>         long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> diff --git a/include/linux/stat.h b/include/linux/stat.h
> index 552e047..75be415 100644
> --- a/include/linux/stat.h
> +++ b/include/linux/stat.h
> @@ -46,4 +46,6 @@ struct kstat {
>         unsigned char   volume_id[16];              /* volume identifier */
>  };
>
> +long xstat_set_result(struct kstat *stat, struct xstat __user *buffer);
> +
>  #endif
> diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
> index 2907352..d7ea6c5 100644
> --- a/include/uapi/linux/stat.h
> +++ b/include/uapi/linux/stat.h
> @@ -90,6 +90,14 @@
>  #define XSTAT_VOLUME_ID        0x00008000U     /* want/got st_volume_id */
>  #define XSTAT_ALL_STATS        0x0000ffffU     /* all supported stats */
>
> +/* xattr request flags */
> +#define XSTAT_XATTR_USER       0x00010000U     /* user.* xattrs */
> +#define XSTAT_XATTR_SYSTEM     0x00020000U     /* system.* xattrs */
> +#define XSTAT_XATTR_SECURITY   0x00040000U     /* security.* xattrs */
> +#define XSTAT_XATTR_POSIX_ACL  0x00080000U     /* posix acl xattrs */
> +#define XSTAT_XATTR_ALL        0x00ff0000U     /* all xattrs */
> +#define XSTAT_XATTR_VALUES     0x01000000U     /* retrieve values along with keys */
> +
>  /*
>   * Extended stat structures
>   */
> @@ -152,4 +160,29 @@ struct xstat {
>  #define XSTAT_INFO_SYSTEM              0x00001000U /* File is marked system (DOS+) */
>  #define XSTAT_INFO_ARCHIVE             0x00002000U /* File is marked archive (DOS+) */
>
> +struct xdirent_xattr {
> +       size_t       xa_value_len;   /* length of value field */
> +       char         xa_name_val[1]; /* name/value pair, name is NULL terminated and
> +                                     * value is xa_value_len in length */
> +};
> +
> +/*
> + * xb_blob: first contains NULL terminated name, followed by struct xdirent_xattr
> + * objects packed together.
> + */
> +struct xdirent_blob {
> +       unsigned int    xb_xattr_count;
> +       char            xb_blob[1]; /* contains variable length data like
> +                                    * NULL-terminated name, xattrs etc */
> +};
> +
> +struct linux_xdirent {
> +       unsigned long        xd_ino;
> +       char                 xd_type;
> +       unsigned long        xd_off;
> +       struct xstat         xd_stat;
> +       unsigned long        xd_reclen;
> +       struct xdirent_blob  xd_blob;
> +};
> +
>  #endif /* _UAPI_LINUX_STAT_H */
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

^ permalink raw reply

* Re: [RFC PATCH 1/2] fs: Add dirreadahead syscall and VFS hooks
From: Michael Kerrisk @ 2014-07-29  8:21 UTC (permalink / raw)
  To: Abhi Das; +Cc: Linux Kernel, Linux-Fsdevel, cluster-devel, Linux API
In-Reply-To: <1406309851-10628-2-git-send-email-adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[CC+=linux-api]

On Fri, Jul 25, 2014 at 7:37 PM, Abhi Das <adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Also adds a void *opaque field to struct dir_context that can be
> used by filesystems to temporarily store any context as this
> struct gets passed around in the fs.
>
> Signed-off-by: Abhi Das <adas-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  arch/x86/syscalls/syscall_32.tbl |  1 +
>  arch/x86/syscalls/syscall_64.tbl |  1 +
>  fs/readdir.c                     | 49 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/fs.h               |  3 +++
>  4 files changed, 54 insertions(+)
>
> diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
> index d6b8679..3e0ef85 100644
> --- a/arch/x86/syscalls/syscall_32.tbl
> +++ b/arch/x86/syscalls/syscall_32.tbl
> @@ -360,3 +360,4 @@
>  351    i386    sched_setattr           sys_sched_setattr
>  352    i386    sched_getattr           sys_sched_getattr
>  353    i386    renameat2               sys_renameat2
> +354    i386    dirreadahead            sys_dirreadahead
> diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
> index ec255a1..2ec0991 100644
> --- a/arch/x86/syscalls/syscall_64.tbl
> +++ b/arch/x86/syscalls/syscall_64.tbl
> @@ -323,6 +323,7 @@
>  314    common  sched_setattr           sys_sched_setattr
>  315    common  sched_getattr           sys_sched_getattr
>  316    common  renameat2               sys_renameat2
> +317    common  dirreadahead            sys_dirreadahead
>
>  #
>  # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 33fd922..d216db7 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -198,6 +198,7 @@ SYSCALL_DEFINE3(getdents, unsigned int, fd,
>         struct linux_dirent __user * lastdirent;
>         struct getdents_callback buf = {
>                 .ctx.actor = filldir,
> +               .ctx.opaque = NULL,
>                 .count = count,
>                 .current_dir = dirent
>         };
> @@ -278,6 +279,7 @@ SYSCALL_DEFINE3(getdents64, unsigned int, fd,
>         struct linux_dirent64 __user * lastdirent;
>         struct getdents_callback64 buf = {
>                 .ctx.actor = filldir64,
> +               .ctx.opaque = NULL,
>                 .count = count,
>                 .current_dir = dirent
>         };
> @@ -304,3 +306,50 @@ SYSCALL_DEFINE3(getdents64, unsigned int, fd,
>         fdput(f);
>         return error;
>  }
> +
> +SYSCALL_DEFINE3(dirreadahead, unsigned int, fd,
> +               loff_t __user *, offset, unsigned int, count)
> +{
> +       struct fd f;
> +       struct inode *inode;
> +       int error = -ENOTDIR;
> +       loff_t off = 0;
> +       struct dir_context ctx = {.actor = NULL, .opaque = NULL};
> +
> +       if (!count)
> +               return -EINVAL;
> +
> +       f = fdget(fd);
> +       if (!f.file)
> +               return -EBADF;
> +
> +       inode = f.file->f_path.dentry->d_inode;
> +
> +       error = -ENOTSUPP;
> +       if (!f.file->f_op || !f.file->f_op->dir_readahead)
> +               goto out;
> +
> +       error = security_file_permission(f.file, MAY_READ);
> +       if (error)
> +               goto out;
> +
> +       error = -EFAULT;
> +       if (__get_user(ctx.pos, offset))
> +               goto out;
> +
> +       error = mutex_lock_killable(&inode->i_mutex);
> +       if (error)
> +               goto out;
> +
> +       error = -ENOENT;
> +       if (!IS_DEADDIR(inode)) {
> +               error = f.file->f_op->dir_readahead(f.file, &ctx, count);
> +               if (__put_user(ctx.pos, offset))
> +                       error = -EFAULT;
> +               file_accessed(f.file);
> +       }
> +       mutex_unlock(&inode->i_mutex);
> +out:
> +       fdput(f);
> +       return error;
> +}
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 338e6f7..fae4a6e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1438,9 +1438,11 @@ int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);
>   * to have different dirent layouts depending on the binary type.
>   */
>  typedef int (*filldir_t)(void *, const char *, int, loff_t, u64, unsigned);
> +
>  struct dir_context {
>         const filldir_t actor;
>         loff_t pos;
> +       void *opaque;
>  };
>
>  struct block_device_operations;
> @@ -1463,6 +1465,7 @@ struct file_operations {
>         ssize_t (*read_iter) (struct kiocb *, struct iov_iter *);
>         ssize_t (*write_iter) (struct kiocb *, struct iov_iter *);
>         int (*iterate) (struct file *, struct dir_context *);
> +       int (*dir_readahead) (struct file *, struct dir_context *, unsigned int);
>         unsigned int (*poll) (struct file *, struct poll_table_struct *);
>         long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
>         long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

^ permalink raw reply

* Hello
From: Noemi Alvarez @ 2014-07-29  8:23 UTC (permalink / raw)



I want to keep up with you with hope for friendship if you are interested.
If you don't mind i will like you to write me back. I am waiting to read
from you, because i have something important and urgent to discuss with
you. I will also send some of my beautiful photos to you.

^ permalink raw reply

* Re: [RFC PATCHv2 00/11] Adding FreeBSD's Capsicum security framework
From: Paolo Bonzini @ 2014-07-29  8:43 UTC (permalink / raw)
  To: Eric W. Biederman, David Drysdale
  Cc: LSM List, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Greg Kroah-Hartman, Alexander Viro, Meredydd Luff, Kees Cook,
	James Morris, Andy Lutomirski, Paul Moore, Christoph Hellwig,
	Linux API
In-Reply-To: <87ha21qja0.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

Il 28/07/2014 23:13, Eric W. Biederman ha scritto:
> This notion that a shared structure should have different semantics
> depending on who is looking at it, sounds like a maintenance nightmare
> to me. 

Isn't that already with seccomp BPF filters?  You can have the parent
process set a BPF filter that forbids read on that file descriptor or
write on that file descriptor.

Effectively, this patchset provides the functionality to "hotplug" BPF
filters on a running process as more file descriptors are passed via
SCM_RIGHTS.  Except it doesn't use BPF filters, and instead uses a
separate set of discrete capabilities.

> I see two sensible implementations:
> - Add a seccomp bpf filter to struct file.

I proposed something like this, but it has some additional
implementation complications.  See the v1 thread.

But it does have to be in a file descriptor rather than a struct file.
It's part of the model that two processes can have different views of
the file descriptor (again my toy example is that of an eventfd that an
unprivileged child process can only write to).

> Additionally unless there is a process wide restriction to relative
> paths I can trivially escape your relative path implementation by simply
> doing open(.) and getting a struct file without any of those
> restrictions.

Yes, there is a prctl for that.

Paolo

^ permalink raw reply

* Re: [RFC PATCHv2 00/11] Adding FreeBSD's Capsicum security framework
From: David Drysdale @ 2014-07-29 10:58 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: LSM List, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
	Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
	Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig,
	Linux API
In-Reply-To: <87ha21qja0.fsf@x220.int.ebiederm.org>

On Mon, Jul 28, 2014 at 10:13 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> David Drysdale <drysdale@google.com> writes:
>
>> On Sat, Jul 26, 2014 at 10:04 PM, Eric W. Biederman
>> <ebiederm@xmission.com> wrote:
>>> David Drysdale <drysdale@google.com> writes:
>
>>>> 1) Capsicum Capabilities
>>>> ------------------------
>>>>
>>>> The most significant aspect of Capsicum is associating *rights* with
>>>> (some) file descriptors, so that the kernel only allows operations on an
>>>> FD if the rights permit it.  This allows userspace applications to
>>>> sandbox themselves by tightly constraining what's allowed with both
>>>> input and outputs; for example, tcpdump might restrict itself so it can
>>>> only read from the network FD, and only write to stdout.
>>>>
>>>> The kernel thus needs to police the rights checks for these file
>>>> descriptors (referred to as 'Capsicum capabilities', completely
>>>> different than POSIX.1e capabilities), and the best place to do this is
>>>> at the points where a file descriptor from userspace is converted to a
>>>> struct file * within the kernel.
>>>>
>>>>   [Policing the rights checks anywhere else, for example at the system
>>>>   call boundary, isn't a good idea because it opens up the possibility
>>>>   of time-of-check/time-of-use (TOCTOU) attacks [2] where FDs are
>>>>   changed (as openat/close/dup2 are allowed in capability mode) between
>>>>   the 'check' at syscall entry and the 'use' at fget() invocation.]
>>>>
>>>> However, this does lead to quite an invasive change to the kernel --
>>>> every invocation of fget() or similar functions (fdget(),
>>>> sockfd_lookup(), user_path_at(),...) needs to be annotated with the
>>>> rights associated with the specific operations that will be performed on
>>>> the struct file.  There are ~100 such invocations that need
>>>> annotation.
>>>
>>> And it is silly.  Roughly you just need a locking version of
>>> fcntl(F_SETFL).
>>>
>>> That is make the restriction in the struct file not in the fd to file
>>> lookup.
>>
>> The file status flags are far too coarse -- for example, O_RDONLY
>> doesn't prevent fchmod(2).
>>
>> Also, because they're associated with the struct file there is no way
>> to pass a file descriptor with only a subset of rights across a UNIX
>> socket (how do I send a read-only FD corresponding to my
>> O_RDWR file?).
>
> You can't.  Unix domain sockets pass struct file references.
> That is passing a file descriptor with a unix domain sockets is the
> equivlanet of dup().

That's my point -- if the rights are associated with the struct file
there is no way of subsetting them.  By associating rights with
the file descriptor, you can pass around different views of the
same underlying object with different rights.

> You absolutely must make your struct file read-only before passing it,
> otherwise the the receiver will have a read-write instance.

Even if the struct file is read-only, the recipient will still be
able to do lots of things (e.g. fchmod as mentioned previously)

> This notion that a shared structure should have different semantics
> depending on who is looking at it, sounds like a maintenance nightmare
> to me.

I don't think it's that bad -- it's not completely different semantics,
just a strict subset of allowed operations.  Also, having a distinct
ENOTCAPABLE errno value makes it easy to spot mismatches
of required & provided rights.

>>> Files in unix have been capabilities for more than 20 years.  That is
>>> what file descriptor passing in unix domain sockets are all about.  We
>>> don't need an additional ``capability rights'' layer on top of file
>>> descriptors.
>>
>> True, the ability to pass FDs across UNIX sockets is one of the
>> key things that makes them analogous to object-capabilities and
>> suitable as the substratum for Capsicum.  But the coarseness of the
>> existing rights, and the lack of coherent policing of those rights,
>> means that an (opt-in) extension like Capsicum is useful.
>
> Finer grained restrictions are perfectly sensible.
>
> I don't see how it make sense to place those restrictions in
> struct fdtable instead of in struct file.
>
> I see two sensible implementations:
> - Add a seccomp bpf filter to struct file.
> - Add permission bits to struct file.
>
> The bpf filter would allow for a simple code extension that would
> allow any arbitrary policy to be applied.

Paolo Bonzini suggested this on the previous iteration, but
I think the associated complications are too overwhelming.
Have a look at: https://lkml.org/lkml/2014/7/7/165

> Adding permisison bits to struct file for gating access to
> file_operations and inode_operations would result in the fastest
> possible implementation and something very easy to audit and
> understand.  But there like ioctl or read/write on IB control files
> where finer grained permissions might be desirable.
>
> For a lot of operations we alread have bits like FMODE_READ and
> FMODE_CAN_READ on struct file for reasons such as performance
> so that only a single cache line needs to be touched.
>
> It might be that it would be worth having both.  Something cheap and
> genrally accessible and something fast.
>
>>> Going farther one huge thing your work and the capsicum work in general
>>> is missing is an implementation of revoke.  With a little care a good
>>> implementation of bits reporting and controlling which methods are
>>> available on which file descriptors should be a good start on
>>> a revoke implementation for linux as well.
>>
>> Yeah, revocation is interesting.  There was been some discussion of
>> it for Capsicum a while ago:
>> https://lists.cam.ac.uk/pipermail/cl-capsicum-discuss/2011-January/msg00002.html
>> but I don't think a firm conclusion was reached.  As I understand it,
>> there's also a theoretical argument that revocation can be constructed
>> on top of the capability system, by handing out capabilities to proxy
>> objects that can then change their behaviour so they no longer pass
>> operations through.
>
> I believe the capsicum approach is so far from allowing general proxy
> objects that, that is not a compelling case.

Yeah, it is probably more of a theoretical argument than a practical
one.  But then, the requirement for revocation might fall into the same
bucket -- more desirable in theory than in practice, particularly for the
sort of application self-sandboxing and compartmentalization that
Capsicum is particularly good for.

>>>> 3) Allowing Capability Mode
>>>> ---------------------------
>>>>
>>>> Capsicum also includes 'capability mode', which locks down the available
>>>> syscalls so the rights restrictions can't just be bypassed by opening
>>>> new file descriptors.  More precisely, capability mode prevents access
>>>> to syscalls that access global namespaces, such as the filesystem or the
>>>> IP:port space.
>>>>
>>>> The existing seccomp-bpf functionality of the kernel is a good mechanism
>>>> for implementing capability mode, but there are a few additional details
>>>> that also need to be addressed.
>>>>
>>>>  a) The capability mode filter program needs to apply process-wide, not
>>>>     just to the current thread.
>>>>
>>>>  b) In capability mode, new files can still be opened with openat(2) but
>>>>     only if they are beneath an existing directory file descriptor.
>>>
>>> Which raises the question is that worth it?
>>
>> From a practical point of view, still allowing (directory-relative) filesystem
>> access means it's much easier for an application to adapt to Capsicum.
>>
>> A simple example is something like unzip/tar -xf, where locking it down
>> is conceptually as simple as:
>>  - apply a CAP_READ restriction to the FD for the input file
>>  - apply a CAP_WRITE+CAP_LOOKUP restriction to the DFD for the
>>    output directory
>>  - enter capability mode.
>> Then the worst a malicious input file can do is to write files under the
>> output directory -- which it could do anyway.
>>
>> Similarly, migrating an application that writes temporary files out to
>> some tempdir is much more straightforward if openat(dfd,...) is still
>> allowed.
>
>
>>>>  c) In capability mode it should still be possible for a process to send
>>>>     signals to itself with kill(2)/tgkill(2).
>
>
>> seL4 may have a much simpler (and more formally-correct) model, but
>> the aim of Capsicum is to get some of the benefits of that well-analysed
>> model without the need for a (massive) migration effort -- and in a way
>> that allows co-existence of migrated and unmigrated code.
>
> Alright. About the same target as seccomp then.  Aiming at something
> that is simple to adopt sounds like a reasonable goal.
>
>>>> 5) New openat(2) O_BENEATH Flag
>>>> -------------------------------
>>>>
>>>> For Capsicum capabilities that are directory file descriptors, the
>>>> Capsicum framework only allows openat(cap_dfd, path, ...) operations to
>>>> work for files that are beneath the specified directory (and even that
>>>> only when the directory FD has the CAP_LOOKUP right), rejecting paths
>>>> that start with "/" or include "..".  The same restriction applies
>>>> process-wide for a process in capability mode.
>>>>
>>>> As this seemed like functionality that might be more generally useful,
>>>> I've implemented it independently as a new O_BENEATH flag for openat(2).
>>>> The Capsicum code then always triggers the use of that flag when the dfd
>>>> is a Capsicum capability, or when the prctl(2) command described above
>>>> is in play.
>>>>
>>>>   [FreeBSD has the openat(2) relative-only behaviour for capability DFDs
>>>>   and processes in capability mode, but does not include the O_BENEATH
>>>>   flag.]
>>>
>>> If you are going to allow open I would think the simple solution here is
>>> to just create a mount namespace that only has available the files you
>>> would like to export to the process, and force all of the file
>>> descriptors into that mount namespace.
>
>> I think that's a lot harder for application writers to do -- they would need
>> to have CAP_SYS_ADMIN to set up the namespace & mounts before
>> dropping privileges.
>
> They only need CAP_SYS_ADMIN in their local user namespace, which anyone
> is allowed to create.
>
>> And the net result would again be much less
>> fine-grained (e.g. for the unzip example above, the specific capability
>> rights prevent reading of any files that already exist in the output
>> directory).
>
> Nope.  What you can implement today if you want fine grained limitations
> like this is to create a mount namespace with exactly the subdirectory
> tree you want to allow access to and to return a file descriptor that
> points into that mount namespace.  (When complete the only user of that
> mount namespace would be your file descriptor).

How does that solve the particular example I mentioned?  The DFD
within the mount namespace will still allow any operation on any file
that's already in the subdirectory -- or am I misunderstanding
something?

> In fact that solution is sufficiently performant and simple that even
> if you came up with a better user space interface for it that is how we
> would want to implement it.
>
> And frankly that already exists today so I think a fairly large burden
> of proof needs to be met to suggest that we need to add additional
> functionality to the kernel.
>
> Furthermore I think it makes sense for application authors to use
> as otherwise they will have to wait a year or so for the debates to be
> finished and your new functionality to be merged.
>
> Additionally unless there is a process wide restriction to relative
> paths I can trivially escape your relative path implementation by simply
> doing open(.) and getting a struct file without any of those
> restrictions.
>
> Eric

Capability mode would impose a process wide restriction to relative
paths -- see sections 3) and 5) of the original mail.

^ permalink raw reply

* Re: [PATCH 0/5] RFC: CGroup Namespaces
From: Andy Lutomirski @ 2014-07-29 15:08 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Linux API, Linux Containers, Serge Hallyn,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
	Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140729045159.GB31047-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>

On Mon, Jul 28, 2014 at 9:51 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Quoting Aditya Kali (adityakali@google.com):
>> Thank you for your review. I have tried to respond to both your emails here.
>>
>> On Thu, Jul 24, 2014 at 9:36 AM, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:
>> > Quoting Aditya Kali (adityakali@google.com):
>> >> Background
>> >>   Cgroups and Namespaces are used together to create “virtual”
>> >>   containers that isolates the host environment from the processes
>> >>   running in container. But since cgroups themselves are not
>> >>   “virtualized”, the task is always able to see global cgroups view
>> >>   through cgroupfs mount and via /proc/self/cgroup file.
>> >>
>> > Hi,
>> >
>> > A few questions/comments:
>> >
>> > 1. Based on this description, am I to understand that after doing a
>> >    cgroupns unshare, 'mount -t cgroup cgroup /mnt' by default will
>> >    still mount the global root cgroup?  Any plans on "changing" that?
>>
>> This is suggested in the "Possible Extensions of CGROUPNS" section.
>> More details below.
>>
>> >    Will attempts to change settings of a cgroup which is not under
>> >    our current ns be rejected?  (That should be easy to do given your
>> >    patch 1/5).  Sorry if it's done in the set, I'm jumping around...
>> >
>>
>> Currently, only 'cgroup_attach_task', 'cgroup_mkdir' and
>> 'cgroup_rmdir' of cgroups outside of cgroupns-root are prevented. The
>> read/write of actual cgroup properties are not prevented. Usual
>> permission checks continue to apply for those. I was hoping that
>> should be enough, but see more comments towards the end.
>>
>> > 2. What would be the reprecussions of allowing cgroupns unshare so
>> >    long as you have ns_capable(CAP_SYS_ADMIN) to the user_ns which
>> >    created your current ns cgroup?  It'd be a shame if that wasn't
>> >    on the roadmap.
>> >
>>
>> Its certainly on the roadmap, just that some logistics were not clear
>> at this time. As pointed out by Andy Lutomirski on [PATCH 5/5] of this
>> series, if we allow cgroupns creation to ns_capable(CAP_SYS_ADMIN)
>> processes, we may need some kind of explicit permission from the
>> cgroup subsystem to allow this. One approach could be an explicit
>
> So long as you do ns_capable(cgroup_ns->user_ns, CAP_SYS_ADMIN) I think
> you're fine.
>
> The only real problem I can think of with unsharing a cgroup_ns is that
> you could lock a setuid-root application someplace it wasn't expecting.
> The above check guarantees that you were privileged enough that you'd
> better be trusted in this user namespace.
>
> (Unless there is some possible interaction I'm overlooking)

I think that, if it's done this way, you'd have to unshare cgroupns
before unsharing userns, since you forfeit that capability when you
unshare your userns.  That means that the new cgroupns ends up being
associated w/ the root userns, which may not be what you want.

You could unshare both namespaces in one syscall and give that some
magic semantics, but that's kind of weird.  It would be nice if you
could unshare your userns and temporarily retains caps in the parent,
but there is no such mechanism right now.

--Andy
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [PATCH 0/5] RFC: CGroup Namespaces
From: Serge E. Hallyn @ 2014-07-29 16:06 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Linux API, Linux Containers, Serge Hallyn,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
	Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CALCETrW5yQLo-SvDgqjt881OD1GnuxMmGKjoohYT4nwtYw=9+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Quoting Andy Lutomirski (luto@amacapital.net):
> On Mon, Jul 28, 2014 at 9:51 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > Quoting Aditya Kali (adityakali@google.com):
> >> Thank you for your review. I have tried to respond to both your emails here.
> >>
> >> On Thu, Jul 24, 2014 at 9:36 AM, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:
> >> > Quoting Aditya Kali (adityakali@google.com):
> >> >> Background
> >> >>   Cgroups and Namespaces are used together to create “virtual”
> >> >>   containers that isolates the host environment from the processes
> >> >>   running in container. But since cgroups themselves are not
> >> >>   “virtualized”, the task is always able to see global cgroups view
> >> >>   through cgroupfs mount and via /proc/self/cgroup file.
> >> >>
> >> > Hi,
> >> >
> >> > A few questions/comments:
> >> >
> >> > 1. Based on this description, am I to understand that after doing a
> >> >    cgroupns unshare, 'mount -t cgroup cgroup /mnt' by default will
> >> >    still mount the global root cgroup?  Any plans on "changing" that?
> >>
> >> This is suggested in the "Possible Extensions of CGROUPNS" section.
> >> More details below.
> >>
> >> >    Will attempts to change settings of a cgroup which is not under
> >> >    our current ns be rejected?  (That should be easy to do given your
> >> >    patch 1/5).  Sorry if it's done in the set, I'm jumping around...
> >> >
> >>
> >> Currently, only 'cgroup_attach_task', 'cgroup_mkdir' and
> >> 'cgroup_rmdir' of cgroups outside of cgroupns-root are prevented. The
> >> read/write of actual cgroup properties are not prevented. Usual
> >> permission checks continue to apply for those. I was hoping that
> >> should be enough, but see more comments towards the end.
> >>
> >> > 2. What would be the reprecussions of allowing cgroupns unshare so
> >> >    long as you have ns_capable(CAP_SYS_ADMIN) to the user_ns which
> >> >    created your current ns cgroup?  It'd be a shame if that wasn't
> >> >    on the roadmap.
> >> >
> >>
> >> Its certainly on the roadmap, just that some logistics were not clear
> >> at this time. As pointed out by Andy Lutomirski on [PATCH 5/5] of this
> >> series, if we allow cgroupns creation to ns_capable(CAP_SYS_ADMIN)
> >> processes, we may need some kind of explicit permission from the
> >> cgroup subsystem to allow this. One approach could be an explicit
> >
> > So long as you do ns_capable(cgroup_ns->user_ns, CAP_SYS_ADMIN) I think
> > you're fine.
> >
> > The only real problem I can think of with unsharing a cgroup_ns is that
> > you could lock a setuid-root application someplace it wasn't expecting.
> > The above check guarantees that you were privileged enough that you'd
> > better be trusted in this user namespace.
> >
> > (Unless there is some possible interaction I'm overlooking)
> 
> I think that, if it's done this way, you'd have to unshare cgroupns
> before unsharing userns, since you forfeit that capability when you
> unshare your userns.  That means that the new cgroupns ends up being
> associated w/ the root userns, which may not be what you want.
> 
> You could unshare both namespaces in one syscall and give that some
> magic semantics, but that's kind of weird.  It would be nice if you
> could unshare your userns and temporarily retains caps in the parent,
> but there is no such mechanism right now.

Hm, good point.
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Does anyone know when FUTEX_WAIT can fail with EAGAIN?
From: Steven Stewart-Gallus @ 2014-07-29 21:55 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-api, mtk

Hello,

I'm trying to debug a hangup where my program loops with FUTEX_WAIT (actually
FUTEX_WAIT_PRIVATE but same thing)  endlessly erring out with EAGAIN. I would
like to know if anyone on the mailing list knows when FUTEX_WAIT can fail with
EAGAIN.

Some info for people who are interested in what I'm working on.

I'm working on sand boxing my program at gitorious.org/linted with namespaces
and chroot.

The error only appeared after changing unshare then fork code into a direct
clone call.

> | uname -a
> Linux alonzo 3.13.0-24-generic #1trisquel1 SMP Tue May 13 15:17:50 UTC 2014
x86_64 x86_64 x86_64 GNU/Linux

Thank you,
Steven Stewart-Gallus

^ permalink raw reply

* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Andy Lutomirski @ 2014-07-30  4:05 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Kees Cook, Julien Tinnes, David Drysdale, Al Viro, Paolo Bonzini,
	LSM List, Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
	Meredydd Luff, Christoph Hellwig,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <87vbqhp4hf.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

On Mon, Jul 28, 2014 at 2:18 PM, Eric W. Biederman
<ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>
>> [cc: Eric Biederman]
>>
>
>> Can we do one better and add a flag to prevent any non-self pid
>> lookups?  This might actually be easy on top of the pid namespace work
>> (e.g. we could change the way that find_task_by_vpid works).
>>
>> It's far from just being signals.  There's access_process_vm, ptrace,
>> all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
>> is ridiculous), and probably some others that I've forgotten about or
>> never noticed in the first place.
>
> So here is the practical question.
>
> Are these processes that only can send signals to their thread group
> allowed to call fork()?
>
>
> If fork is allowed and all pid lookups are restricted to their own
> thread group that wait, waitpid, and all of the rest of the wait family
> will never return the pids of their children, and zombies will
> accumulate.  Aka the semantics are fundamentally broken.

Good point.

I can imagine at least three ways that fork() could continue working, though:

1. Allow lookups of immediate children, too.  (I don't love this one.)
2. Allow non-self pids to be translated in but not out.  This way
P_ALL will continue working.
3. Have the kernel treat any PID-restricted process as though it were NOCLDWAIT.

I think I like #3.  Thoughts?

>
> If fork is not allowed pid namespaces already solve this problem.

PID namespaces are fairly heavyweight.  Julien pointed out that using
PID namespaces requires a bunch of dummy PID 1 processes.

--Andy

^ permalink raw reply

* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Eric W. Biederman @ 2014-07-30  4:08 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Kees Cook, Julien Tinnes, David Drysdale, Al Viro, Paolo Bonzini,
	LSM List, Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
	Meredydd Luff, Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrWaUsi1Ea3YTXLN6BFqcoHnbFTuMvcNncS5rq0nSgOatA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:

> On Mon, Jul 28, 2014 at 2:18 PM, Eric W. Biederman
> <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>>
>>> [cc: Eric Biederman]
>>>
>>
>>> Can we do one better and add a flag to prevent any non-self pid
>>> lookups?  This might actually be easy on top of the pid namespace work
>>> (e.g. we could change the way that find_task_by_vpid works).
>>>
>>> It's far from just being signals.  There's access_process_vm, ptrace,
>>> all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
>>> is ridiculous), and probably some others that I've forgotten about or
>>> never noticed in the first place.
>>
>> So here is the practical question.
>>
>> Are these processes that only can send signals to their thread group
>> allowed to call fork()?
>>
>>
>> If fork is allowed and all pid lookups are restricted to their own
>> thread group that wait, waitpid, and all of the rest of the wait family
>> will never return the pids of their children, and zombies will
>> accumulate.  Aka the semantics are fundamentally broken.
>
> Good point.
>
> I can imagine at least three ways that fork() could continue working, though:
>
> 1. Allow lookups of immediate children, too.  (I don't love this one.)
> 2. Allow non-self pids to be translated in but not out.  This way
> P_ALL will continue working.
> 3. Have the kernel treat any PID-restricted process as though it were NOCLDWAIT.
>
> I think I like #3.  Thoughts?
>
>>
>> If fork is not allowed pid namespaces already solve this problem.
>
> PID namespaces are fairly heavyweight.  Julien pointed out that using
> PID namespaces requires a bunch of dummy PID 1 processes.

Only if you can't tolerate init exiting.  The reasoning with respect to
signals and signals being ignored was wrong.  And if you only have one
process you care about and no children to worry about neither the
difference in signal handling nor the world dies whe init exits applies.

Therefore given what I have read described pid namespaces are a trivial
solution to this problem space.

Eric

^ permalink raw reply

* Re: [PATCH 11/11] seccomp: Add tgid and tid into seccomp_data
From: Andy Lutomirski @ 2014-07-30  4:35 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Kees Cook, Julien Tinnes, David Drysdale, Al Viro, Paolo Bonzini,
	LSM List, Greg Kroah-Hartman, Paul Moore, James Morris, Linux API,
	Meredydd Luff, Christoph Hellwig, linux-kernel@vger.kernel.org
In-Reply-To: <87oaw7ij4k.fsf@x220.int.ebiederm.org>

On Tue, Jul 29, 2014 at 9:08 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Andy Lutomirski <luto@amacapital.net> writes:
>
>> On Mon, Jul 28, 2014 at 2:18 PM, Eric W. Biederman
>> <ebiederm@xmission.com> wrote:
>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>
>>>> [cc: Eric Biederman]
>>>>
>>>
>>>> Can we do one better and add a flag to prevent any non-self pid
>>>> lookups?  This might actually be easy on top of the pid namespace work
>>>> (e.g. we could change the way that find_task_by_vpid works).
>>>>
>>>> It's far from just being signals.  There's access_process_vm, ptrace,
>>>> all the signal functions, clock_gettime (see CPUCLOCK_PID -- yes, this
>>>> is ridiculous), and probably some others that I've forgotten about or
>>>> never noticed in the first place.
>>>
>>> So here is the practical question.
>>>
>>> Are these processes that only can send signals to their thread group
>>> allowed to call fork()?
>>>
>>>
>>> If fork is allowed and all pid lookups are restricted to their own
>>> thread group that wait, waitpid, and all of the rest of the wait family
>>> will never return the pids of their children, and zombies will
>>> accumulate.  Aka the semantics are fundamentally broken.
>>
>> Good point.
>>
>> I can imagine at least three ways that fork() could continue working, though:
>>
>> 1. Allow lookups of immediate children, too.  (I don't love this one.)
>> 2. Allow non-self pids to be translated in but not out.  This way
>> P_ALL will continue working.
>> 3. Have the kernel treat any PID-restricted process as though it were NOCLDWAIT.
>>
>> I think I like #3.  Thoughts?
>>
>>>
>>> If fork is not allowed pid namespaces already solve this problem.
>>
>> PID namespaces are fairly heavyweight.  Julien pointed out that using
>> PID namespaces requires a bunch of dummy PID 1 processes.
>
> Only if you can't tolerate init exiting.  The reasoning with respect to
> signals and signals being ignored was wrong.  And if you only have one
> process you care about and no children to worry about neither the
> difference in signal handling nor the world dies whe init exits applies.

Can you elaborate?  It seems entirely plausible to me that there are
programs that won't work right as PID 1 without considerable
adaptation.

>
> Therefore given what I have read described pid namespaces are a trivial
> solution to this problem space.

pid namespaces also won't work in the context of Capsicum unless you
want every single Capsicum process to be its own pid namespace.  Also,
pid namespaces don't offer any way to protect children from parents.

--Andy

^ permalink raw reply

* Re: [RFC PATCHv2 00/11] Adding FreeBSD's Capsicum security framework
From: Eric W. Biederman @ 2014-07-30  6:22 UTC (permalink / raw)
  To: David Drysdale
  Cc: LSM List, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
	Alexander Viro, Meredydd Luff, Kees Cook, James Morris,
	Andy Lutomirski, Paolo Bonzini, Paul Moore, Christoph Hellwig,
	Linux API
In-Reply-To: <CAHse=S_w4+AMuc=-XbAK_PiaD56_ks13R53RENMHif5KRN_Kiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


I have cut this down to just focus on O_BENEATH openat case.

David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> writes:

> On Mon, Jul 28, 2014 at 10:13 PM, Eric W. Biederman
> <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:

>> Nope.  What you can implement today if you want fine grained limitations
>> like this is to create a mount namespace with exactly the subdirectory
>> tree you want to allow access to and to return a file descriptor that
>> points into that mount namespace.  (When complete the only user of that
>> mount namespace would be your file descriptor).
>
> How does that solve the particular example I mentioned?  The DFD
> within the mount namespace will still allow any operation on any file
> that's already in the subdirectory -- or am I misunderstanding
> something?

The goal was to bound the DFD to the directory and all of it's
subdirectories such that openat(dfd, "../../..") would open
the dfd, and that further opens of other directories would also not
allow you to escape.

Since the mount namespace only contains the choosen directory and it's
subdirectories that works easily and trivially.

So while you can indeed perform any file operation on that dfd who
cares because none of those operations can get you anywhere you aren't
supposed to be.

My point was that you can as granular as you would like by binding a dfd
to a mount namespace instead of binding a process to a mount namespace,
and the code already exists and is being maintained.

So while things are not packaged in the form that has been requested it
looks to me as if the functionality for directories already exists
within the Linux kernel.

Eric

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox