10:06 a.m.
On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman <ebiederm(a)xmission.com> wrote:
Linus,
Please pull the for-linus branch from the git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git for-linus
HEAD: 344470cac42e887e68cfb5bdfa6171baf27f1eb5 proc: Point /proc/mounts at
/proc/thread-self/mounts instead of /proc/self/mounts
This is a bunch of small changes built against 3.16-rc6. The most
significant change for users is the first patch which makes setns
drmatically faster by removing unneded rcu handling.
The next chunk of changes are so that "mount -o remount,.." will not
allow the user namespace root to drop flags on a mount set by the system
wide root. Aks this forces read-only mounts to stay read-only, no-dev
mounts to stay no-dev, no-suid mounts to stay no-suid, no-exec mounts to
stay no exec and it prevents unprivileged users from messing with a
mounts atime settings. I have included my test case as the last patch
in this series so people performing backports can verify this change
works correctly.
The next change fixes a bug in NFS that was discovered while auditing
nsproxy users for the first optimization. Today you can oops the kernel
by reading /proc/fs/nfsfs/{servers,volumes} if you are clever with pid
namespaces. I rebased and fixed the build of the !CONFIG_NFS_FS case
yesterday when a build bot caught my typo. Given that no one to my
knowledge bases anything on my tree fixing the typo in place seems more
responsible that requiring a typo-fix to be backported as well.
The last change is a small semantic cleanup introducing
/proc/thread-self and pointing /proc/mounts and /proc/net at it. This
prevents several kinds of problemantic corner cases. It is a
user-visible change so it has a minute chance of causing regressions so
the change to /proc/mounts and /proc/net are individual one line commits
that can be trivially reverted. Unfortunately I lost and could not find
the email of the original reporter so he is not credited. From at least
one perspective this change to /proc/net is a refgression fix to allow
pthread /proc/net uses that were broken by the introduction of the network
namespace.
Eric
Eric W. Biederman (11):
namespaces: Use task_lock and not rcu to protect nsproxy
mnt: Only change user settable mount flags in remount
mnt: Move the test for MNT_LOCK_READONLY from change_mount_flags into do_remount
mnt: Correct permission checks in do_remount
This commit breaks libvirt-lxc.
libvirt does in lxcContainerMountBasicFS():
/*
* We can't immediately set the MS_RDONLY flag when mounting filesystems
* because (in at least some kernel versions) this will propagate back
* to the original mount in the host OS, turning it readonly too. Thus
* we mount the filesystem in read-write mode initially, and then do a
* separate read-only bind mount on top of that.
*/
bindOverReadonly = !!(mnt_mflags & MS_RDONLY);
VIR_DEBUG("Mount %s on %s type=%s flags=%x",
mnt_src, mnt->dst, mnt->type, mnt_mflags & ~MS_RDONLY);
if (mount(mnt_src, mnt->dst, mnt->type, mnt_mflags &
~MS_RDONLY, NULL) < 0) {
^^^^ Here it fails for sysfs because with user namespaces we bind the
existing /sys into the container
and would have to read out all existing mount flags from the current /sys mount.
Otherwise mount() fails with EPERM.
On my test system /sys is mounted with
"rw,nosuid,nodev,noexec,relatime" and libvirt
misses the realtime...
virReportSystemError(errno,
_("Failed to mount %s on %s type %s
flags=%x"),
mnt_src, mnt->dst, NULLSTR(mnt->type),
mnt_mflags & ~MS_RDONLY);
goto cleanup;
}
if (bindOverReadonly &&
mount(mnt_src, mnt->dst, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) {
^^^ Here it fails because now we'd have to specify all flags as used
for the first
mount. For the procfs case MS_NOSUID|MS_NOEXEC|MS_NODEV.
See lxcBasicMounts[].
In this case the fix is easy, add mnt_mflags to the mount flags.
virReportSystemError(errno,
_("Failed to re-mount %s on %s flags=%x"),
mnt_src, mnt->dst,
MS_BIND|MS_REMOUNT|MS_RDONLY);
goto cleanup;
}
--
Thanks,
//richard
11:53 p.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Richard Weinberger <richard.weinberger(a)gmail.com> writes:
On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman
<ebiederm(a)xmission.com> wrote:
This commit breaks libvirt-lxc.
libvirt does in lxcContainerMountBasicFS():
The bugs fixed are security issues, so if we have to break a small
number of userspace applications we will. Anything that we can
reasonably do to avoid regressions will be done.
Could you please look at my user-namespace.git#for-next branch I have a
fix for at least one regresion causing issue in there. I think it may
fix your issues but I am not fully certain more comments below.
/*
* We can't immediately set the MS_RDONLY flag when mounting filesystems
* because (in at least some kernel versions) this will propagate back
* to the original mount in the host OS, turning it readonly too. Thus
* we mount the filesystem in read-write mode initially, and then do a
* separate read-only bind mount on top of that.
*/
bindOverReadonly = !!(mnt_mflags & MS_RDONLY);
VIR_DEBUG("Mount %s on %s type=%s flags=%x",
mnt_src, mnt->dst, mnt->type, mnt_mflags & ~MS_RDONLY);
if (mount(mnt_src, mnt->dst, mnt->type, mnt_mflags &
~MS_RDONLY, NULL) < 0) {
^^^^ Here it fails for sysfs because with user namespaces we bind the
existing /sys into the container
and would have to read out all existing mount flags from the current /sys mount.
Otherwise mount() fails with EPERM.
On my test system /sys is mounted with
"rw,nosuid,nodev,noexec,relatime" and libvirt
misses the realtime...
Not specifying any atime flags to mount should be safe as that asks for
the default atime flags which for remount I have made the default atime
flags the existing atime flags. So I am scratching my head a little on
this one.
virReportSystemError(errno,
_("Failed to mount %s on %s type %s
flags=%x"),
mnt_src, mnt->dst, NULLSTR(mnt->type),
mnt_mflags & ~MS_RDONLY);
goto cleanup;
}
if (bindOverReadonly &&
mount(mnt_src, mnt->dst, NULL,
MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) {
^^^ Here it fails because now we'd have to specify all flags as used
for the first
mount. For the procfs case MS_NOSUID|MS_NOEXEC|MS_NODEV.
See lxcBasicMounts[].
In this case the fix is easy, add mnt_mflags to the mount flags.
That has always been a bug in general because remount has always
required specifying the complete set of mount flags you want to have.
That fact that flags such as nosuid are now properly locked so you can
not change them if you are not the global root user just makes this
obvious.
Andy Lutermorski has observed that statvfs will return the mount flags
making reading them simple.
Eric
1:29 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Am 21.08.2014 06:53, schrieb Eric W. Biederman:
The bugs fixed are security issues, so if we have to break a small
number of userspace applications we will. Anything that we can
reasonably do to avoid regressions will be done.
Could you please look at my user-namespace.git#for-next branch I have a
fix for at least one regresion causing issue in there. I think it may
fix your issues but I am not fully certain more comments below.
I'll run this on my LXC testbed today.
> /*
> * We can't immediately set the MS_RDONLY flag when mounting filesystems
> * because (in at least some kernel versions) this will propagate back
> * to the original mount in the host OS, turning it readonly too. Thus
> * we mount the filesystem in read-write mode initially, and then do a
> * separate read-only bind mount on top of that.
> */
> bindOverReadonly = !!(mnt_mflags & MS_RDONLY);
>
> VIR_DEBUG("Mount %s on %s type=%s flags=%x",
> mnt_src, mnt->dst, mnt->type, mnt_mflags & ~MS_RDONLY);
> if (mount(mnt_src, mnt->dst, mnt->type, mnt_mflags &
> ~MS_RDONLY, NULL) < 0) {
>
> ^^^^ Here it fails for sysfs because with user namespaces we bind the
> existing /sys into the container
> and would have to read out all existing mount flags from the current /sys mount.
> Otherwise mount() fails with EPERM.
> On my test system /sys is mounted with
> "rw,nosuid,nodev,noexec,relatime" and libvirt
> misses the realtime...
Not specifying any atime flags to mount should be safe as that asks for
the default atime flags which for remount I have made the default atime
flags the existing atime flags. So I am scratching my head a little on
this one.
Okay, let me find out why exactly libvirt gets a EPERM here.
Maybe there are more odds hidden.
>
> virReportSystemError(errno,
> _("Failed to mount %s on %s type %s
flags=%x"),
> mnt_src, mnt->dst, NULLSTR(mnt->type),
> mnt_mflags & ~MS_RDONLY);
> goto cleanup;
> }
>
> if (bindOverReadonly &&
> mount(mnt_src, mnt->dst, NULL,
> MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) {
>
> ^^^ Here it fails because now we'd have to specify all flags as used
> for the first
> mount. For the procfs case MS_NOSUID|MS_NOEXEC|MS_NODEV.
> See lxcBasicMounts[].
> In this case the fix is easy, add mnt_mflags to the mount flags.
That has always been a bug in general because remount has always
required specifying the complete set of mount flags you want to have.
That fact that flags such as nosuid are now properly locked so you can
not change them if you are not the global root user just makes this
obvious.
Andy Lutermorski has observed that statvfs will return the mount flags
making reading them simple.
Thanks for the clarification, I'll create a fix for libvirt.
Thanks,
//richard
2:24 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Am 21.08.2014 08:29, schrieb Richard Weinberger:
Am 21.08.2014 06:53, schrieb Eric W. Biederman:
> The bugs fixed are security issues, so if we have to break a small
> number of userspace applications we will. Anything that we can
> reasonably do to avoid regressions will be done.
>
> Could you please look at my user-namespace.git#for-next branch I have a
> fix for at least one regresion causing issue in there. I think it may
> fix your issues but I am not fully certain more comments below.
I'll run this on my LXC testbed today.
Looks good. With these patches applied libvirt works again. :)
Thanks,
//richard
8:54 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Richard Weinberger <richard(a)nod.at> writes:
Am 21.08.2014 08:29, schrieb Richard Weinberger:
> Am 21.08.2014 06:53, schrieb Eric W. Biederman:
>> The bugs fixed are security issues, so if we have to break a small
>> number of userspace applications we will. Anything that we can
>> reasonably do to avoid regressions will be done.
>>
>> Could you please look at my user-namespace.git#for-next branch I have a
>> fix for at least one regresion causing issue in there. I think it may
>> fix your issues but I am not fully certain more comments below.
>
> I'll run this on my LXC testbed today.
Looks good. With these patches applied libvirt works again. :)
Darn I read my email in the wrong order. I am glad to hear that my
changes were enough to fix libvirt-lxc.
I will aim at pushing this to Linus after the conference is over and I
can trust myself to think clearly.
Eric
8:12 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
On Wed, Aug 20, 2014 at 09:53:49PM -0700, Eric W. Biederman wrote:
Richard Weinberger <richard.weinberger(a)gmail.com> writes:
> On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman <ebiederm(a)xmission.com>
wrote:
>
> This commit breaks libvirt-lxc.
> libvirt does in lxcContainerMountBasicFS():
The bugs fixed are security issues, so if we have to break a small
number of userspace applications we will. Anything that we can
reasonably do to avoid regressions will be done.
Can you explain the security issues in detail? Breaking common
userspace like libvirt-lxc with just a little bit of handwaiving is
entirely unacceptable.
8:22 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Am 21.08.2014 15:12, schrieb Christoph Hellwig:
On Wed, Aug 20, 2014 at 09:53:49PM -0700, Eric W. Biederman wrote:
> Richard Weinberger <richard.weinberger(a)gmail.com> writes:
>
>> On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman <ebiederm(a)xmission.com>
wrote:
>>
>> This commit breaks libvirt-lxc.
>> libvirt does in lxcContainerMountBasicFS():
>
> The bugs fixed are security issues, so if we have to break a small
> number of userspace applications we will. Anything that we can
> reasonably do to avoid regressions will be done.
Can you explain the security issues in detail? Breaking common
userspace like libvirt-lxc with just a little bit of handwaiving is
entirely unacceptable.
It looks like commit 87b47932f40a11280584bce260cbdb3b5f9e8b7d in
git.kernel.org/cgit/linux/kernel/git/ebiederm/user-namespace.git for-next
unbreaks libvirt-lxc.
I hope it hits Linus tree and -stable before the offending commit hits users.
Thanks,
//richard
9:09 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Richard Weinberger <richard(a)nod.at> writes:
Am 21.08.2014 15:12, schrieb Christoph Hellwig:
> On Wed, Aug 20, 2014 at 09:53:49PM -0700, Eric W. Biederman wrote:
>> Richard Weinberger <richard.weinberger(a)gmail.com> writes:
>>
>>> On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman
<ebiederm(a)xmission.com> wrote:
>>>
>>> This commit breaks libvirt-lxc.
>>> libvirt does in lxcContainerMountBasicFS():
>>
>> The bugs fixed are security issues, so if we have to break a small
>> number of userspace applications we will. Anything that we can
>> reasonably do to avoid regressions will be done.
>
> Can you explain the security issues in detail? Breaking common
> userspace like libvirt-lxc with just a little bit of handwaiving is
> entirely unacceptable.
It looks like commit 87b47932f40a11280584bce260cbdb3b5f9e8b7d in
git.kernel.org/cgit/linux/kernel/git/ebiederm/user-namespace.git for-next
unbreaks libvirt-lxc.
I hope it hits Linus tree and -stable before the offending commit hits users.
I plan to send the pull request to Linus as soon as I have caught my
breath (from all of the conferences this week) that I can be certain I
am thinking clearly and not rushing things.
Eric
4:18 p.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
On Thu, Aug 21, 2014 at 4:09 PM, Eric W. Biederman
<ebiederm(a)xmission.com> wrote:
> It looks like commit 87b47932f40a11280584bce260cbdb3b5f9e8b7d in
> git.kernel.org/cgit/linux/kernel/git/ebiederm/user-namespace.git for-next
> unbreaks libvirt-lxc.
> I hope it hits Linus tree and -stable before the offending commit hits users.
I plan to send the pull request to Linus as soon as I have caught my
breath (from all of the conferences this week) that I can be certain I
am thinking clearly and not rushing things.
*kind reminder* :-)
--
Thanks,
//richard
5:15 p.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Eric,
On Thu, Aug 21, 2014 at 4:09 PM, Eric W. Biederman
<ebiederm(a)xmission.com> wrote:
Richard Weinberger <richard(a)nod.at> writes:
> Am 21.08.2014 15:12, schrieb Christoph Hellwig:
>> On Wed, Aug 20, 2014 at 09:53:49PM -0700, Eric W. Biederman wrote:
>>> Richard Weinberger <richard.weinberger(a)gmail.com> writes:
>>>
>>>> On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman
<ebiederm(a)xmission.com> wrote:
>>>>
>>>> This commit breaks libvirt-lxc.
>>>> libvirt does in lxcContainerMountBasicFS():
>>>
>>> The bugs fixed are security issues, so if we have to break a small
>>> number of userspace applications we will. Anything that we can
>>> reasonably do to avoid regressions will be done.
>>
>> Can you explain the security issues in detail? Breaking common
>> userspace like libvirt-lxc with just a little bit of handwaiving is
>> entirely unacceptable.
>
> It looks like commit 87b47932f40a11280584bce260cbdb3b5f9e8b7d in
> git.kernel.org/cgit/linux/kernel/git/ebiederm/user-namespace.git for-next
> unbreaks libvirt-lxc.
> I hope it hits Linus tree and -stable before the offending commit hits users.
I plan to send the pull request to Linus as soon as I have caught my
breath (from all of the conferences this week) that I can be certain I
am thinking clearly and not rushing things.
Today I've upgraded my LXC testbed to the most recent kernel and found
libvirt-lxc broken again (sic!).
Remounting /proc/sys/ is failing.
Investigating into the issue showed that commit "mnt: Implicitly add
MNT_NODEV on remount as we do on mount"
is not mainline.
Why did you left out this patch? In my previous mails I explicitly
stated that exactly this commit unbreaks libvirt-lxc.
Now the userspace breaking changes are mainline and hit users hard. :-(
--
Thanks,
//richard
5:34 p.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
[just replying on libvir-list]
Am 26. Nov 2014, 00:15 schrieb Richard Weinberger <richard.weinberger(a)gmail.com>:
> I plan to send the pull request to Linus as soon as I have caught
my
> breath (from all of the conferences this week) that I can be certain I
> am thinking clearly and not rushing things.
Today I've upgraded my LXC testbed to the most recent kernel and found
libvirt-lxc broken again (sic!).
Remounting /proc/sys/ is failing.
Investigating into the issue showed that commit "mnt: Implicitly add
MNT_NODEV on remount as we do on mount"
is not mainline.
Why did you left out this patch? In my previous mails I explicitly
stated that exactly this commit unbreaks libvirt-lxc.
Now the userspace breaking changes are mainline and hit users hard. :-(
I can confirm that userns with libvirt-lxc and 3.17-rc* is broken and
the (kernel) commit in question fixes the issue.
Best,
Matthias
10:58 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Am 26.11.2014 um 00:15 schrieb Richard Weinberger:
Eric,
On Thu, Aug 21, 2014 at 4:09 PM, Eric W. Biederman
<ebiederm(a)xmission.com> wrote:
> Richard Weinberger <richard(a)nod.at> writes:
>
>> Am 21.08.2014 15:12, schrieb Christoph Hellwig:
>>> On Wed, Aug 20, 2014 at 09:53:49PM -0700, Eric W. Biederman wrote:
>>>> Richard Weinberger <richard.weinberger(a)gmail.com> writes:
>>>>
>>>>> On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman
<ebiederm(a)xmission.com> wrote:
>>>>>
>>>>> This commit breaks libvirt-lxc.
>>>>> libvirt does in lxcContainerMountBasicFS():
>>>>
>>>> The bugs fixed are security issues, so if we have to break a small
>>>> number of userspace applications we will. Anything that we can
>>>> reasonably do to avoid regressions will be done.
>>>
>>> Can you explain the security issues in detail? Breaking common
>>> userspace like libvirt-lxc with just a little bit of handwaiving is
>>> entirely unacceptable.
>>
>> It looks like commit 87b47932f40a11280584bce260cbdb3b5f9e8b7d in
>> git.kernel.org/cgit/linux/kernel/git/ebiederm/user-namespace.git for-next
>> unbreaks libvirt-lxc.
>> I hope it hits Linus tree and -stable before the offending commit hits users.
>
> I plan to send the pull request to Linus as soon as I have caught my
> breath (from all of the conferences this week) that I can be certain I
> am thinking clearly and not rushing things.
Today I've upgraded my LXC testbed to the most recent kernel and found
libvirt-lxc broken again (sic!).
Remounting /proc/sys/ is failing.
Investigating into the issue showed that commit "mnt: Implicitly add
MNT_NODEV on remount as we do on mount"
is not mainline.
Why did you left out this patch? In my previous mails I explicitly
stated that exactly this commit unbreaks libvirt-lxc.
Now the userspace breaking changes are mainline and hit users hard. :-(
*kind ping*
...to make sure that this issue doesn't get lost.
Thanks,
//richard
8:43 a.m.
New subject: [libvirt] [GIT PULL] namespace updates for v3.17-rc1
Christoph Hellwig <hch(a)infradead.org> writes:
On Wed, Aug 20, 2014 at 09:53:49PM -0700, Eric W. Biederman wrote:
> Richard Weinberger <richard.weinberger(a)gmail.com> writes:
>
> > On Wed, Aug 6, 2014 at 2:57 AM, Eric W. Biederman <ebiederm(a)xmission.com>
wrote:
> >
> > This commit breaks libvirt-lxc.
> > libvirt does in lxcContainerMountBasicFS():
>
> The bugs fixed are security issues, so if we have to break a small
> number of userspace applications we will. Anything that we can
> reasonably do to avoid regressions will be done.
Can you explain the security issues in detail? Breaking common
userspace like libvirt-lxc with just a little bit of handwaiving is
entirely unacceptable.
The biggies ability for an unprivileged user to clear nosuid, nodev,
read-only, noexec with remount.
Apparently part of what libvirt-lxc clearing all mount flags except for
read-only on some filesystem it remounts read-only. Which if root
mounted the filesystem with nosuid, nodev, or noexec is not supportable.
So if it isn't the implicit setting of nodev on mount but not on remount
causing problems (which I have the trivial fix for) there is a strong
chance I need to break libvirt-lxc.
I am several bugs deep already where fixing one bug reveals yet another
bug. It is possible there is something else that that I have not
discovered that I am missing. I am more to happy to investigate to see
if it possible to avoid breaking libvirt-lxc.
Apologies for being a little vague being at the conference I only have
access to email about an hour a day.
Eric
3678
days inactive
3779
days old
12 comments
5 participants
participants (5)
-
Christoph Hellwig -
Eric W. Biederman -
Matthias Maier -
Richard Weinberger -
Richard Weinberger