| Message ID | 20200827170947.429611-1-zwisler@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v9,1/2] Add a "nosymfollow" mount option. | expand |
On Thu, Aug 27, 2020 at 11:09:46AM -0600, Ross Zwisler wrote: > From: Mattias Nissler <mnissler@chromium.org> > > For mounts that have the new "nosymfollow" option, don't follow symlinks > when resolving paths. The new option is similar in spirit to the > existing "nodev", "noexec", and "nosuid" options, as well as to the > LOOKUP_NO_SYMLINKS resolve flag in the openat2(2) syscall. Various BSD > variants have been supporting the "nosymfollow" mount option for a long > time with equivalent implementations. > > Note that symlinks may still be created on file systems mounted with > the "nosymfollow" option present. readlink() remains functional, so > user space code that is aware of symlinks can still choose to follow > them explicitly. > > Setting the "nosymfollow" mount option helps prevent privileged > writers from modifying files unintentionally in case there is an > unexpected link along the accessed path. The "nosymfollow" option is > thus useful as a defensive measure for systems that need to deal with > untrusted file systems in privileged contexts. > > More information on the history and motivation for this patch can be > found here: > > https://sites.google.com/a/chromium.org/dev/chromium-os/chromiumos-design-docs/hardening-against-malicious-stateful-data#TOC-Restricting-symlink-traversal > > Signed-off-by: Mattias Nissler <mnissler@chromium.org> > Signed-off-by: Ross Zwisler <zwisler@google.com> > Reviewed-by: Aleksa Sarai <cyphar@cyphar.com> > --- > Changes since v8 [1]: > * Look for MNT_NOSYMFOLLOW in link->mnt->mnt_flags so we are testing > the link itself rather than the directory holding the link. (Al Viro) > * Rebased onto v5.9-rc2. AFAICS, it applies clean to -rc1; what was the rebase about?
On Thu, Aug 27, 2020 at 09:08:01PM +0100, Al Viro wrote: > On Thu, Aug 27, 2020 at 11:09:46AM -0600, Ross Zwisler wrote: > > From: Mattias Nissler <mnissler@chromium.org> > > > > For mounts that have the new "nosymfollow" option, don't follow symlinks > > when resolving paths. The new option is similar in spirit to the > > existing "nodev", "noexec", and "nosuid" options, as well as to the > > LOOKUP_NO_SYMLINKS resolve flag in the openat2(2) syscall. Various BSD > > variants have been supporting the "nosymfollow" mount option for a long > > time with equivalent implementations. > > > > Note that symlinks may still be created on file systems mounted with > > the "nosymfollow" option present. readlink() remains functional, so > > user space code that is aware of symlinks can still choose to follow > > them explicitly. > > > > Setting the "nosymfollow" mount option helps prevent privileged > > writers from modifying files unintentionally in case there is an > > unexpected link along the accessed path. The "nosymfollow" option is > > thus useful as a defensive measure for systems that need to deal with > > untrusted file systems in privileged contexts. > > > > More information on the history and motivation for this patch can be > > found here: > > > > https://sites.google.com/a/chromium.org/dev/chromium-os/chromiumos-design-docs/hardening-against-malicious-stateful-data#TOC-Restricting-symlink-traversal > > > > Signed-off-by: Mattias Nissler <mnissler@chromium.org> > > Signed-off-by: Ross Zwisler <zwisler@google.com> > > Reviewed-by: Aleksa Sarai <cyphar@cyphar.com> > > --- > > Changes since v8 [1]: > > * Look for MNT_NOSYMFOLLOW in link->mnt->mnt_flags so we are testing > > the link itself rather than the directory holding the link. (Al Viro) > > * Rebased onto v5.9-rc2. > > AFAICS, it applies clean to -rc1; what was the rebase about? Applied (to -rc1) and pushed
On Thu, Aug 27, 2020 at 09:10:15PM +0100, Al Viro wrote: > On Thu, Aug 27, 2020 at 09:08:01PM +0100, Al Viro wrote: > > AFAICS, it applies clean to -rc1; what was the rebase about? Oh, sorry if that was confusing, I just wanted to make sure that it still applied cleanly to the latest -rc so that you didn't hit a merge conflict. Yes, these patches apply cleanly to both -rc1 and -rc2. > Applied (to -rc1) and pushed Many thanks!
On Thu, Aug 27, 2020 at 2:25 PM Ross Zwisler <zwisler@google.com> wrote: > On Thu, Aug 27, 2020 at 09:10:15PM +0100, Al Viro wrote: > > On Thu, Aug 27, 2020 at 09:08:01PM +0100, Al Viro wrote: > > Applied (to -rc1) and pushed > > Many thanks! (apologies for the resend, the previous one had HTML and was rejected by the lists) Just FYI, here is the related commit in upstream util-linux: https://github.com/karelzak/util-linux/commit/50a531f667c31d54fbb920d394e6008df89ae636 and the thread to linux-man, which I will ping when the v5.10 merge window closes: https://lore.kernel.org/linux-man/CAKgNAkiAkyUjd=cUvASaT2tyhaCdiMF48KA3Ov_1mQf0=J2PXw@mail.gmail.com/
diff --git a/fs/namei.c b/fs/namei.c index e99e2a9da0f7d..33e8c79bc761e 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1626,7 +1626,8 @@ static const char *pick_link(struct nameidata *nd, struct path *link, return ERR_PTR(error); } - if (unlikely(nd->flags & LOOKUP_NO_SYMLINKS)) + if (unlikely(nd->flags & LOOKUP_NO_SYMLINKS) || + unlikely(link->mnt->mnt_flags & MNT_NOSYMFOLLOW)) return ERR_PTR(-ELOOP); if (!(nd->flags & LOOKUP_RCU)) { diff --git a/fs/namespace.c b/fs/namespace.c index bae0e95b3713a..6408788a649e1 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -3160,6 +3160,8 @@ int path_mount(const char *dev_name, struct path *path, mnt_flags &= ~(MNT_RELATIME | MNT_NOATIME); if (flags & MS_RDONLY) mnt_flags |= MNT_READONLY; + if (flags & MS_NOSYMFOLLOW) + mnt_flags |= MNT_NOSYMFOLLOW; /* The default atime for remount is preservation */ if ((flags & MS_REMOUNT) && diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c index 3059a9394c2d6..e59d4bb3a89e4 100644 --- a/fs/proc_namespace.c +++ b/fs/proc_namespace.c @@ -70,6 +70,7 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt) { MNT_NOATIME, ",noatime" }, { MNT_NODIRATIME, ",nodiratime" }, { MNT_RELATIME, ",relatime" }, + { MNT_NOSYMFOLLOW, ",nosymfollow" }, { 0, NULL } }; const struct proc_fs_opts *fs_infop; diff --git a/fs/statfs.c b/fs/statfs.c index 2616424012ea7..59f33752c1311 100644 --- a/fs/statfs.c +++ b/fs/statfs.c @@ -29,6 +29,8 @@ static int flags_by_mnt(int mnt_flags) flags |= ST_NODIRATIME; if (mnt_flags & MNT_RELATIME) flags |= ST_RELATIME; + if (mnt_flags & MNT_NOSYMFOLLOW) + flags |= ST_NOSYMFOLLOW; return flags; } diff --git a/include/linux/mount.h b/include/linux/mount.h index de657bd211fa6..aaf343b38671c 100644 --- a/include/linux/mount.h +++ b/include/linux/mount.h @@ -30,6 +30,7 @@ struct fs_context; #define MNT_NODIRATIME 0x10 #define MNT_RELATIME 0x20 #define MNT_READONLY 0x40 /* does the user want this to be r/o? */ +#define MNT_NOSYMFOLLOW 0x80 #define MNT_SHRINKABLE 0x100 #define MNT_WRITE_HOLD 0x200 @@ -46,7 +47,7 @@ struct fs_context; #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_NOSYMFOLLOW) #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 9bc69edb8f188..fac4356ea1bfc 100644 --- a/include/linux/statfs.h +++ b/include/linux/statfs.h @@ -40,6 +40,7 @@ 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_NOSYMFOLLOW 0x2000 /* do not follow symlinks */ struct dentry; extern int vfs_get_fsid(struct dentry *dentry, __kernel_fsid_t *fsid); diff --git a/include/uapi/linux/mount.h b/include/uapi/linux/mount.h index 96a0240f23fed..dd8306ea336c1 100644 --- a/include/uapi/linux/mount.h +++ b/include/uapi/linux/mount.h @@ -16,6 +16,7 @@ #define MS_REMOUNT 32 /* Alter flags of a mounted FS */ #define MS_MANDLOCK 64 /* Allow mandatory locks on an FS */ #define MS_DIRSYNC 128 /* Directory modifications are synchronous */ +#define MS_NOSYMFOLLOW 256 /* Do not follow symlinks */ #define MS_NOATIME 1024 /* Do not update access times. */ #define MS_NODIRATIME 2048 /* Do not update directory access times */ #define MS_BIND 4096