| Message ID | 874mmodral.fsf_-_@x220.int.ebiederm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 03, 2015 at 11:35:30PM -0500, Eric W. Biederman wrote: > > Not allowing programs to clear nosuid and noexec on new mounts of > sysfs or proc will cause lxc and libvirt-lxc to fail to start (a > regression). There are no executables files on sysfs or proc today > which means clearing these flags is harmless today. > > Instead of failing the fresh mounts of sysfs and proc emit a warning > when these flags are improprely cleared. We only reach this point > because lxc and libvirt-lxc clear flags they mount flags had not > intended to. > > In a couple of kernel releases when lxc and libvirt-lxc have been > fixed we can start failing fresh mounts proc and sysfs that clear > nosuid and noexec. Userspace clearly means to enforce those > attributes and enforcing these attributes have historically avoided > bugs in the setattr implementations of proc and sysfs. > > Cc: stable@vger.kernel.org > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > > Now with warning on problematic remounts as well. > nodev is also ignored because it is not currently problematic. > > fs/namespace.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/mount.h | 5 +++++ > 2 files changed, 38 insertions(+) > > diff --git a/fs/namespace.c b/fs/namespace.c > index eccd925c6e82..3c3f8172c734 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -2162,6 +2162,18 @@ static int do_remount(struct path *path, int flags, int mnt_flags, > ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) { > return -EPERM; > } > + if ((mnt->mnt.mnt_flags & MNT_WARN_NOSUID) && > + !(mnt_flags & MNT_NOSUID) && printk_ratelimit()) { > + printk(KERN_INFO > + "warning: process `%s' clears nosuid in remount of %s\n", > + current->comm, sb->s_type->name); > + } > + if ((mnt->mnt.mnt_flags & MNT_WARN_NOEXEC) && > + !(mnt_flags & MNT_NOEXEC) && printk_ratelimit()) { > + printk(KERN_INFO > + "warning: process `%s' clears noexec in remount of %s\n", > + current->comm, sb->s_type->name); > + } > > err = security_sb_remount(sb, data); > if (err) > @@ -3201,12 +3213,14 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags) > if ((mnt->mnt.mnt_flags & MNT_LOCK_NODEV) && > !(new_flags & MNT_NODEV)) > continue; > +#if 0 /* Avoid unnecessary regressions */ > if ((mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) && > !(new_flags & MNT_NOSUID)) > continue; > if ((mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) && > !(new_flags & MNT_NOEXEC)) > continue; > +#endif > if ((mnt->mnt.mnt_flags & MNT_LOCK_ATIME) && > ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (new_flags & MNT_ATIME_MASK))) > continue; > @@ -3227,9 +3241,28 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags) > /* Preserve the locked attributes */ > *new_mnt_flags |= mnt->mnt.mnt_flags & (MNT_LOCK_READONLY | \ > MNT_LOCK_NODEV | \ > + /* Avoid unnecessary regressions \ > MNT_LOCK_NOSUID | \ > MNT_LOCK_NOEXEC | \ > + */ \ > MNT_LOCK_ATIME); > + /* For now, warn about the "harmless" but invalid mnt flags */ > + if (mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) { > + *new_mnt_flags |= MNT_WARN_NOSUID; > + if (!(new_flags & MNT_NOSUID) && printk_ratelimit()) { > + printk(KERN_INFO > + "warning: process `%s' clears nosuid in mount of %s\n", > + current->comm, type->name); > + } > + } > + if (mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) { > + *new_mnt_flags |= MNT_WARN_NOEXEC; > + if (!(new_flags & MNT_NOEXEC) && printk_ratelimit()) { > + printk(KERN_INFO > + "warning: process `%s' clears noexec in mount of %s\n", > + current->comm, type->name); > + } > + } Adding this to a stable kernel is not going to be ok, sorry. We can't start being noisy in system logs for things that were working just fine. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/namespace.c b/fs/namespace.c index eccd925c6e82..3c3f8172c734 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2162,6 +2162,18 @@ static int do_remount(struct path *path, int flags, int mnt_flags, ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (mnt_flags & MNT_ATIME_MASK))) { return -EPERM; } + if ((mnt->mnt.mnt_flags & MNT_WARN_NOSUID) && + !(mnt_flags & MNT_NOSUID) && printk_ratelimit()) { + printk(KERN_INFO + "warning: process `%s' clears nosuid in remount of %s\n", + current->comm, sb->s_type->name); + } + if ((mnt->mnt.mnt_flags & MNT_WARN_NOEXEC) && + !(mnt_flags & MNT_NOEXEC) && printk_ratelimit()) { + printk(KERN_INFO + "warning: process `%s' clears noexec in remount of %s\n", + current->comm, sb->s_type->name); + } err = security_sb_remount(sb, data); if (err) @@ -3201,12 +3213,14 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags) if ((mnt->mnt.mnt_flags & MNT_LOCK_NODEV) && !(new_flags & MNT_NODEV)) continue; +#if 0 /* Avoid unnecessary regressions */ if ((mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) && !(new_flags & MNT_NOSUID)) continue; if ((mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) && !(new_flags & MNT_NOEXEC)) continue; +#endif if ((mnt->mnt.mnt_flags & MNT_LOCK_ATIME) && ((mnt->mnt.mnt_flags & MNT_ATIME_MASK) != (new_flags & MNT_ATIME_MASK))) continue; @@ -3227,9 +3241,28 @@ static bool fs_fully_visible(struct file_system_type *type, int *new_mnt_flags) /* Preserve the locked attributes */ *new_mnt_flags |= mnt->mnt.mnt_flags & (MNT_LOCK_READONLY | \ MNT_LOCK_NODEV | \ + /* Avoid unnecessary regressions \ MNT_LOCK_NOSUID | \ MNT_LOCK_NOEXEC | \ + */ \ MNT_LOCK_ATIME); + /* For now, warn about the "harmless" but invalid mnt flags */ + if (mnt->mnt.mnt_flags & MNT_LOCK_NOSUID) { + *new_mnt_flags |= MNT_WARN_NOSUID; + if (!(new_flags & MNT_NOSUID) && printk_ratelimit()) { + printk(KERN_INFO + "warning: process `%s' clears nosuid in mount of %s\n", + current->comm, type->name); + } + } + if (mnt->mnt.mnt_flags & MNT_LOCK_NOEXEC) { + *new_mnt_flags |= MNT_WARN_NOEXEC; + if (!(new_flags & MNT_NOEXEC) && printk_ratelimit()) { + printk(KERN_INFO + "warning: process `%s' clears noexec in mount of %s\n", + current->comm, type->name); + } + } visible = true; goto found; next: ; diff --git a/include/linux/mount.h b/include/linux/mount.h index f822c3c11377..a9ac188413fd 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -52,6 +52,11 @@ struct mnt_namespace; #define MNT_INTERNAL 0x4000 +/* These warning options should be removed in a few kernel releases + * once userspace has been fixed. + */ +#define MNT_WARN_NOSUID 0x010000 +#define MNT_WARN_NOEXEC 0x020000 #define MNT_LOCK_ATIME 0x040000 #define MNT_LOCK_NOEXEC 0x080000 #define MNT_LOCK_NOSUID 0x100000
Not allowing programs to clear nosuid and noexec on new mounts of sysfs or proc will cause lxc and libvirt-lxc to fail to start (a regression). There are no executables files on sysfs or proc today which means clearing these flags is harmless today. Instead of failing the fresh mounts of sysfs and proc emit a warning when these flags are improprely cleared. We only reach this point because lxc and libvirt-lxc clear flags they mount flags had not intended to. In a couple of kernel releases when lxc and libvirt-lxc have been fixed we can start failing fresh mounts proc and sysfs that clear nosuid and noexec. Userspace clearly means to enforce those attributes and enforcing these attributes have historically avoided bugs in the setattr implementations of proc and sysfs. Cc: stable@vger.kernel.org Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- Now with warning on problematic remounts as well. nodev is also ignored because it is not currently problematic. fs/namespace.c | 33 +++++++++++++++++++++++++++++++++ include/linux/mount.h | 5 +++++ 2 files changed, 38 insertions(+)