Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754212AbcKPVS5 (ORCPT ); Wed, 16 Nov 2016 16:18:57 -0500 Received: from mail-vk0-f47.google.com ([209.85.213.47]:35998 "EHLO mail-vk0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752809AbcKPVSy (ORCPT ); Wed, 16 Nov 2016 16:18:54 -0500 MIME-Version: 1.0 In-Reply-To: References: <1476880312-64786-1-git-send-email-mnissler@chromium.org> From: Mattias Nissler Date: Wed, 16 Nov 2016 13:18:32 -0800 X-Google-Sender-Auth: qLmO8j0GXZBPVyqIRGbh_k4y4b4 Message-ID: Subject: Re: [PATCH v2] Add a "nosymlinks" mount option. To: linux-fsdevel@vger.kernel.org Cc: Alexander Viro , linux-kernel@vger.kernel.org, Colin Walters , "Austin S . Hemmelgarn" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6774 Lines: 155 I understand that silence suggests there's little interest, but here's some new information I discovered today that may justify to reconsider the patch: The BSDs already have exactly what I propose, the mount option is called "nosymfollow" there: https://github.com/freebsd/freebsd/blob/a41f4cc9a57cd74604ae7b051eec2f48865f18d6/sys/kern/vfs_lookup.c#L939 There's also some evidence on the net that people have been using said nosymfollow mount option to mitigate symlink attacks. On Mon, Oct 24, 2016 at 7:09 AM, Mattias Nissler wrote: > Friendly ping - does this version of the patch have any chance on > getting included in mainline? > > On Wed, Oct 19, 2016 at 2:31 PM, Mattias Nissler wrote: >> For mounts that have the new "nosymlinks" option, don't follow >> symlinks when resolving paths. The new option is similar in spirit to >> the existing "nodev", "noexec", and "nosuid" options. >> >> Note that symlinks may still be created on mounts where the >> "nosymlinks" option is present. readlink() remains functional, so user >> space code that is aware of symlinks can still choose to follow them >> explicitly. >> >> Setting the "nosymlinks" mount option helps prevent privileged writers >> from modifying files unintentionally in case there is an unexpected >> link along the accessed path. The "nosymlinks" option is thus useful >> as a defensive measure for systems that need to deal with untrusted >> file systems in privileged contexts. >> >> Signed-off-by: Mattias Nissler >> --- >> fs/namei.c | 3 +++ >> fs/namespace.c | 9 ++++++--- >> fs/proc_namespace.c | 1 + >> fs/statfs.c | 2 ++ >> include/linux/mount.h | 3 ++- >> include/linux/statfs.h | 1 + >> include/uapi/linux/fs.h | 1 + >> 7 files changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/fs/namei.c b/fs/namei.c >> index 5b4eed2..4cddcf3 100644 >> --- a/fs/namei.c >> +++ b/fs/namei.c >> @@ -1021,6 +1021,9 @@ const char *get_link(struct nameidata *nd) >> touch_atime(&last->link); >> } >> >> + if (nd->path.mnt->mnt_flags & MNT_NOSYMLINKS) >> + return ERR_PTR(-EACCES); >> + >> error = security_inode_follow_link(dentry, inode, >> nd->flags & LOOKUP_RCU); >> if (unlikely(error)) >> diff --git a/fs/namespace.c b/fs/namespace.c >> index e6c234b..deec84e 100644 >> --- a/fs/namespace.c >> +++ b/fs/namespace.c >> @@ -2732,6 +2732,8 @@ long do_mount(const char *dev_name, const char __user *dir_name, >> mnt_flags &= ~(MNT_RELATIME | MNT_NOATIME); >> if (flags & MS_RDONLY) >> mnt_flags |= MNT_READONLY; >> + if (flags & MS_NOSYMLINKS) >> + mnt_flags |= MNT_NOSYMLINKS; >> >> /* The default atime for remount is preservation */ >> if ((flags & MS_REMOUNT) && >> @@ -2741,9 +2743,10 @@ long do_mount(const char *dev_name, const char __user *dir_name, >> mnt_flags |= path.mnt->mnt_flags & MNT_ATIME_MASK; >> } >> >> - flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_BORN | >> - MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT | >> - MS_STRICTATIME | MS_NOREMOTELOCK); >> + flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_NOSYMLINKS | >> + MS_ACTIVE | MS_BORN | MS_NOATIME | MS_NODIRATIME | >> + MS_RELATIME | MS_KERNMOUNT | MS_STRICTATIME | >> + MS_NOREMOTELOCK); >> >> if (flags & MS_REMOUNT) >> retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags, >> diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c >> index 3f1190d..a1949d9 100644 >> --- a/fs/proc_namespace.c >> +++ b/fs/proc_namespace.c >> @@ -67,6 +67,7 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt) >> { MNT_NOATIME, ",noatime" }, >> { MNT_NODIRATIME, ",nodiratime" }, >> { MNT_RELATIME, ",relatime" }, >> + { MNT_NOSYMLINKS, ",nosymlinks" }, >> { 0, NULL } >> }; >> const struct proc_fs_info *fs_infop; >> diff --git a/fs/statfs.c b/fs/statfs.c >> index 083dc0a..7ff7c32 100644 >> --- a/fs/statfs.c >> +++ b/fs/statfs.c >> @@ -27,6 +27,8 @@ static int flags_by_mnt(int mnt_flags) >> flags |= ST_NODIRATIME; >> if (mnt_flags & MNT_RELATIME) >> flags |= ST_RELATIME; >> + if (mnt_flags & MNT_NOSYMLINKS) >> + flags |= ST_NOSYMLINKS; >> return flags; >> } >> >> diff --git a/include/linux/mount.h b/include/linux/mount.h >> index 1172cce..5e302f4 100644 >> --- a/include/linux/mount.h >> +++ b/include/linux/mount.h >> @@ -28,6 +28,7 @@ struct mnt_namespace; >> #define MNT_NODIRATIME 0x10 >> #define MNT_RELATIME 0x20 >> #define MNT_READONLY 0x40 /* does the user want this to be r/o? */ >> +#define MNT_NOSYMLINKS 0x80 >> >> #define MNT_SHRINKABLE 0x100 >> #define MNT_WRITE_HOLD 0x200 >> @@ -44,7 +45,7 @@ struct mnt_namespace; >> #define MNT_SHARED_MASK (MNT_UNBINDABLE) >> #define MNT_USER_SETTABLE_MASK (MNT_NOSUID | MNT_NODEV | MNT_NOEXEC \ >> | MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME \ >> - | MNT_READONLY) >> + | MNT_READONLY | MNT_NOSYMLINKS) >> #define MNT_ATIME_MASK (MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME ) >> >> #define MNT_INTERNAL_FLAGS (MNT_SHARED | MNT_WRITE_HOLD | MNT_INTERNAL | \ >> diff --git a/include/linux/statfs.h b/include/linux/statfs.h >> index 0166d32..994b059 100644 >> --- a/include/linux/statfs.h >> +++ b/include/linux/statfs.h >> @@ -39,5 +39,6 @@ struct kstatfs { >> #define ST_NOATIME 0x0400 /* do not update access times */ >> #define ST_NODIRATIME 0x0800 /* do not update directory access times */ >> #define ST_RELATIME 0x1000 /* update atime relative to mtime/ctime */ >> +#define ST_NOSYMLINKS 0x2000 /* do not follow symbolic links */ >> >> #endif >> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h >> index acb2b61..06193d8 100644 >> --- a/include/uapi/linux/fs.h >> +++ b/include/uapi/linux/fs.h >> @@ -130,6 +130,7 @@ struct inodes_stat_t { >> #define MS_I_VERSION (1<<23) /* Update inode I_version field */ >> #define MS_STRICTATIME (1<<24) /* Always perform atime updates */ >> #define MS_LAZYTIME (1<<25) /* Update the on-disk [acm]times lazily */ >> +#define MS_NOSYMLINKS (1<<26) /* Do not follow symbolic links */ >> >> /* These sb flags are internal to the kernel */ >> #define MS_NOREMOTELOCK (1<<27) >> -- >> 2.8.0.rc3.226.g39d4020 >>