On Tue, May 2, 2017 at 6:33 PM, Andy Lutomirski [off-list ref] wrote:

On Tue, May 2, 2017 at 7:29 AM, Djalal Harouni [off-list ref] wrote:

quoted

On Thu, Apr 27, 2017 at 12:13 AM, Andy Lutomirski [off-list ref] wrote:

quoted

On Tue, Apr 25, 2017 at 5:23 AM, Djalal Harouni [off-list ref] wrote:

[...]

quoted

quoted

We have to align procfs and modernize it to have a per mount context
where at least the mount option do not propagate to all other mounts,
then maybe we can continue to implement new features. One example is to
require CAP_SYS_ADMIN in the init user namespace on some /proc/* which are
not pids and which are are not virtualized by design, or CAP_NET_ADMIN
inside userns on the net bits that are virtualized, etc.
These mount options won't propagate to previous mounts, and the system
will continue to be usable.

Ths patch introduces the new 'limit_pids' mount option as it was also
suggesed by Andy Lutomirski [1]. When this option is passed we
automatically create a private procfs instance. This is not the default
behaviour since we do not want to break userspace and we do not want to
provide different devices IDs by default, please see [1] for why.

I think that calling the option to make a separate instance
"limit_pids" is extremely counterintuitive.

Ok.

quoted

My strong preference would be to make proc *always* make a separate
instance (unless it's a bind mount) and to make it work.  If that
means fudging stat() output, so be it.

I also agree, but as said if we change stat(), userspace won't be able
to notice if these two proc instances are really separated, the device
ID is the only indication here.

I re-read all the threads and I'm still not convinced I see why we
need new_instance to be non-default.  It's true that the device
numbers of /proc/ns/* matter, but if you look (with stat -L, for
example), they're *already* not tied to the procfs instance.

Hmm, indeed, so the namespace FDs point internally to the internal
proc mount that is created during pidns initialization, this means
NS_GET_PARENT ioctl won't change which is good, only things that
relate on stat()ing other inodes may notice.

I'm okay with adding new_instance to be on the safe side, but I'd like
it to be done in a way that we could make it become the default some
day without breaking anything.  This means that we need to be rather
careful about how new_instance and hidepid interact.

Sounds good, from the devpts history it seems that "newinstance" was
used to absorb new changes/updates easily, and it was made a no-op
only recently with commit eedf265aa003b4 "devpts: Make each mount of
devpts an independent filesystem."  last year, where the initial
introduction was via commit 2a1b2dc0c83bbfc24 "Enable multiple
instances of devpts"  in 2009

Starting from this: 1) "hidepid" works withe the "gid" membership
option which is sticky, I would like to avoid this combination, plus
2) "hidepid" now changes the pid namespace option.

With "newinstance" set:

* "hidepid" instead of changing the pid namespace options, it will
only affect the new procfs instance.

* Changing "hidepid" value during a remount of a *private* procfs
instance will only affect that procfs instance and not the pid
namespace or the other shared procfs mounts.

* "pids=ptraceable" makes /proc/ show only pids that the caller can
ptrace. Together with NO_NEW_PRIVS set, it makes a good privacy
measure.
"pids=ptraceable" is also for *LSM* so we guarantee that there is a
ptrace security hook there for LSMs and that there are no relations or
exceptions between "pids=ptraceable" and "hidepid" / "gid" mount
options. This will benefit Yama LSM later.

* "pids=ptraceable" will take precedence over "hidepid"


I assume defaulting later to new instances should continue to work, comments ?


Thanks!

-- 
tixxdz