From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [CFT][PATCH 11/10] mnt: Avoid unnecessary regressions in fs_fully_visible (take 2) Date: Thu, 4 Jun 2015 14:20:39 +0900 Message-ID: <20150604052039.GB21049@kroah.com> References: <87lhg8pwvz.fsf@x220.int.ebiederm.org> <87fv6gikfn.fsf@x220.int.ebiederm.org> <87fv6g80g7.fsf@x220.int.ebiederm.org> <87k2vkebri.fsf@x220.int.ebiederm.org> <87eglseboh.fsf_-_@x220.int.ebiederm.org> <874mmodral.fsf_-_@x220.int.ebiederm.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <874mmodral.fsf_-_-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org> Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Eric W. Biederman" Cc: Andy Lutomirski , Kenton Varda , Serge Hallyn , Seth Forshee , Linux API , Linux Containers , Michael Kerrisk-manpages , Richard Weinberger , Linux FS Devel , Tejun Heo List-Id: linux-api@vger.kernel.org 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Signed-off-by: "Eric W. Biederman" > --- > > 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