diff options
author | Kees Cook <keescook@chromium.org> | 2015-09-04 15:44:57 -0700 |
---|---|---|
committer | Ben Hutchings <ben@decadent.org.uk> | 2015-10-13 03:46:08 +0100 |
commit | f4a08180fbf920c796e8ab8e186efa08238545dd (patch) | |
tree | c842b4ab53e691629326fec98b6e8154da00855e | |
parent | 3af9b38d21e276dc168e229e6cde2a28cdadf328 (diff) | |
download | linux-stable-f4a08180fbf920c796e8ab8e186efa08238545dd.tar.gz linux-stable-f4a08180fbf920c796e8ab8e186efa08238545dd.tar.bz2 linux-stable-f4a08180fbf920c796e8ab8e186efa08238545dd.zip |
fs: create and use seq_show_option for escaping
commit a068acf2ee77693e0bf39d6e07139ba704f461c3 upstream.
Many file systems that implement the show_options hook fail to correctly
escape their output which could lead to unescaped characters (e.g. new
lines) leaking into /proc/mounts and /proc/[pid]/mountinfo files. This
could lead to confusion, spoofed entries (resulting in things like
systemd issuing false d-bus "mount" notifications), and who knows what
else. This looks like it would only be the root user stepping on
themselves, but it's possible weird things could happen in containers or
in other situations with delegated mount privileges.
Here's an example using overlay with setuid fusermount trusting the
contents of /proc/mounts (via the /etc/mtab symlink). Imagine the use
of "sudo" is something more sneaky:
$ BASE="ovl"
$ MNT="$BASE/mnt"
$ LOW="$BASE/lower"
$ UP="$BASE/upper"
$ WORK="$BASE/work/ 0 0
none /proc fuse.pwn user_id=1000"
$ mkdir -p "$LOW" "$UP" "$WORK"
$ sudo mount -t overlay -o "lowerdir=$LOW,upperdir=$UP,workdir=$WORK" none /mnt
$ cat /proc/mounts
none /root/ovl/mnt overlay rw,relatime,lowerdir=ovl/lower,upperdir=ovl/upper,workdir=ovl/work/ 0 0
none /proc fuse.pwn user_id=1000 0 0
$ fusermount -u /proc
$ cat /proc/mounts
cat: /proc/mounts: No such file or directory
This fixes the problem by adding new seq_show_option and
seq_show_option_n helpers, and updating the vulnerable show_option
handlers to use them as needed. Some, like SELinux, need to be open
coded due to unusual existing escape mechanisms.
[akpm@linux-foundation.org: add lost chunk, per Kees]
[keescook@chromium.org: seq_show_option should be using const parameters]
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Acked-by: Jan Kara <jack@suse.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Cc: J. R. Okajima <hooanon05g@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[bwh: Backported to 3.2:
- Drop changes to overlayfs, reiserfs
- Drop vers option from cifs
- ceph changes are all in one file
- Adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
-rw-r--r-- | fs/ceph/super.c | 8 | ||||
-rw-r--r-- | fs/cifs/cifsfs.c | 4 | ||||
-rw-r--r-- | fs/ext4/super.c | 4 | ||||
-rw-r--r-- | fs/gfs2/super.c | 6 | ||||
-rw-r--r-- | fs/hfs/super.c | 4 | ||||
-rw-r--r-- | fs/hfsplus/options.c | 4 | ||||
-rw-r--r-- | fs/hostfs/hostfs_kern.c | 2 | ||||
-rw-r--r-- | fs/ocfs2/super.c | 4 | ||||
-rw-r--r-- | fs/xfs/xfs_super.c | 4 | ||||
-rw-r--r-- | include/linux/seq_file.h | 35 | ||||
-rw-r--r-- | kernel/cgroup.c | 7 | ||||
-rw-r--r-- | security/selinux/hooks.c | 2 |
12 files changed, 61 insertions, 23 deletions
diff --git a/fs/ceph/super.c b/fs/ceph/super.c index 3c981db4996a..8bfafe54997a 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -361,8 +361,10 @@ static int ceph_show_options(struct seq_file *m, struct vfsmount *mnt) if (opt->flags & CEPH_OPT_NOCRC) seq_puts(m, ",nocrc"); - if (opt->name) - seq_printf(m, ",name=%s", opt->name); + if (opt->name) { + seq_puts(m, ",name="); + seq_escape(m, opt->name, ", \t\n\\"); + } if (opt->key) seq_puts(m, ",secret=<hidden>"); @@ -405,7 +407,7 @@ static int ceph_show_options(struct seq_file *m, struct vfsmount *mnt) if (fsopt->max_readdir_bytes != CEPH_MAX_READDIR_BYTES_DEFAULT) seq_printf(m, ",readdir_max_bytes=%d", fsopt->max_readdir_bytes); if (strcmp(fsopt->snapdir_name, CEPH_SNAPDIRNAME_DEFAULT)) - seq_printf(m, ",snapdirname=%s", fsopt->snapdir_name); + seq_show_option(m, "snapdirname", fsopt->snapdir_name); return 0; } diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 25bb97fc0d54..d955b8e6c467 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -381,10 +381,10 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m) if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER) seq_printf(s, ",multiuser"); else if (tcon->ses->user_name) - seq_printf(s, ",username=%s", tcon->ses->user_name); + seq_show_option(s, "username", tcon->ses->user_name); if (tcon->ses->domainName) - seq_printf(s, ",domain=%s", tcon->ses->domainName); + seq_show_option(s, "domain", tcon->ses->domainName); if (srcaddr->sa_family != AF_UNSPEC) { struct sockaddr_in *saddr4; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index be4db0e13493..e2cf43bc689f 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1018,10 +1018,10 @@ static inline void ext4_show_quota_options(struct seq_file *seq, } if (sbi->s_qf_names[USRQUOTA]) - seq_printf(seq, ",usrjquota=%s", sbi->s_qf_names[USRQUOTA]); + seq_show_option(seq, "usrjquota", sbi->s_qf_names[USRQUOTA]); if (sbi->s_qf_names[GRPQUOTA]) - seq_printf(seq, ",grpjquota=%s", sbi->s_qf_names[GRPQUOTA]); + seq_show_option(seq, "grpjquota", sbi->s_qf_names[GRPQUOTA]); if (test_opt(sb, USRQUOTA)) seq_puts(seq, ",usrquota"); diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index 71e420989f77..be2ece552018 100644 --- a/fs/gfs2/super.c +++ b/fs/gfs2/super.c @@ -1298,11 +1298,11 @@ static int gfs2_show_options(struct seq_file *s, struct vfsmount *mnt) if (is_ancestor(mnt->mnt_root, sdp->sd_master_dir)) seq_printf(s, ",meta"); if (args->ar_lockproto[0]) - seq_printf(s, ",lockproto=%s", args->ar_lockproto); + seq_show_option(s, "lockproto", args->ar_lockproto); if (args->ar_locktable[0]) - seq_printf(s, ",locktable=%s", args->ar_locktable); + seq_show_option(s, "locktable", args->ar_locktable); if (args->ar_hostdata[0]) - seq_printf(s, ",hostdata=%s", args->ar_hostdata); + seq_show_option(s, "hostdata", args->ar_hostdata); if (args->ar_spectator) seq_printf(s, ",spectator"); if (args->ar_localflocks) diff --git a/fs/hfs/super.c b/fs/hfs/super.c index 1b55f704fb22..cac813d9d2cb 100644 --- a/fs/hfs/super.c +++ b/fs/hfs/super.c @@ -138,9 +138,9 @@ static int hfs_show_options(struct seq_file *seq, struct vfsmount *mnt) struct hfs_sb_info *sbi = HFS_SB(mnt->mnt_sb); if (sbi->s_creator != cpu_to_be32(0x3f3f3f3f)) - seq_printf(seq, ",creator=%.4s", (char *)&sbi->s_creator); + seq_show_option_n(seq, "creator", (char *)&sbi->s_creator, 4); if (sbi->s_type != cpu_to_be32(0x3f3f3f3f)) - seq_printf(seq, ",type=%.4s", (char *)&sbi->s_type); + seq_show_option_n(seq, "type", (char *)&sbi->s_type, 4); seq_printf(seq, ",uid=%u,gid=%u", sbi->s_uid, sbi->s_gid); if (sbi->s_file_umask != 0133) seq_printf(seq, ",file_umask=%o", sbi->s_file_umask); diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c index bb62a5882147..c8d6b4fc271b 100644 --- a/fs/hfsplus/options.c +++ b/fs/hfsplus/options.c @@ -211,9 +211,9 @@ int hfsplus_show_options(struct seq_file *seq, struct vfsmount *mnt) struct hfsplus_sb_info *sbi = HFSPLUS_SB(mnt->mnt_sb); if (sbi->creator != HFSPLUS_DEF_CR_TYPE) - seq_printf(seq, ",creator=%.4s", (char *)&sbi->creator); + seq_show_option_n(seq, "creator", (char *)&sbi->creator, 4); if (sbi->type != HFSPLUS_DEF_CR_TYPE) - seq_printf(seq, ",type=%.4s", (char *)&sbi->type); + seq_show_option_n(seq, "type", (char *)&sbi->type, 4); seq_printf(seq, ",umask=%o,uid=%u,gid=%u", sbi->umask, sbi->uid, sbi->gid); if (sbi->part >= 0) diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 2f72da5ae686..104e4d9b4067 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -265,7 +265,7 @@ static int hostfs_show_options(struct seq_file *seq, struct vfsmount *vfs) size_t offset = strlen(root_ino) + 1; if (strlen(root_path) > offset) - seq_printf(seq, ",%s", root_path + offset); + seq_show_option(seq, root_path + offset, NULL); return 0; } diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index 4994f8b0e604..5fe6b1ef6bcd 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -1583,8 +1583,8 @@ static int ocfs2_show_options(struct seq_file *s, struct vfsmount *mnt) seq_printf(s, ",localflocks,"); if (osb->osb_cluster_stack[0]) - seq_printf(s, ",cluster_stack=%.*s", OCFS2_STACK_LABEL_LEN, - osb->osb_cluster_stack); + seq_show_option_n(s, "cluster_stack", osb->osb_cluster_stack, + OCFS2_STACK_LABEL_LEN); if (opts & OCFS2_MOUNT_USRQUOTA) seq_printf(s, ",usrquota"); if (opts & OCFS2_MOUNT_GRPQUOTA) diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 8a899496fd5f..90ccd1c4782c 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -533,9 +533,9 @@ xfs_showargs( seq_printf(m, "," MNTOPT_LOGBSIZE "=%dk", mp->m_logbsize >> 10); if (mp->m_logname) - seq_printf(m, "," MNTOPT_LOGDEV "=%s", mp->m_logname); + seq_show_option(m, MNTOPT_LOGDEV, mp->m_logname); if (mp->m_rtname) - seq_printf(m, "," MNTOPT_RTDEV "=%s", mp->m_rtname); + seq_show_option(m, MNTOPT_RTDEV, mp->m_rtname); if (mp->m_dalign > 0) seq_printf(m, "," MNTOPT_SUNIT "=%d", diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h index 0b69a4684216..8803d6e8068c 100644 --- a/include/linux/seq_file.h +++ b/include/linux/seq_file.h @@ -122,6 +122,41 @@ void *__seq_open_private(struct file *, const struct seq_operations *, int); int seq_open_private(struct file *, const struct seq_operations *, int); int seq_release_private(struct inode *, struct file *); +/** + * seq_show_options - display mount options with appropriate escapes. + * @m: the seq_file handle + * @name: the mount option name + * @value: the mount option name's value, can be NULL + */ +static inline void seq_show_option(struct seq_file *m, const char *name, + const char *value) +{ + seq_putc(m, ','); + seq_escape(m, name, ",= \t\n\\"); + if (value) { + seq_putc(m, '='); + seq_escape(m, value, ", \t\n\\"); + } +} + +/** + * seq_show_option_n - display mount options with appropriate escapes + * where @value must be a specific length. + * @m: the seq_file handle + * @name: the mount option name + * @value: the mount option name's value, cannot be NULL + * @length: the length of @value to display + * + * This is a macro since this uses "length" to define the size of the + * stack buffer. + */ +#define seq_show_option_n(m, name, value, length) { \ + char val_buf[length + 1]; \ + strncpy(val_buf, value, length); \ + val_buf[length] = '\0'; \ + seq_show_option(m, name, val_buf); \ +} + #define SEQ_START_TOKEN ((void *)1) /* diff --git a/kernel/cgroup.c b/kernel/cgroup.c index eafb6dd3e19c..ec64a1823a50 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -1053,15 +1053,16 @@ static int cgroup_show_options(struct seq_file *seq, struct vfsmount *vfs) mutex_lock(&cgroup_mutex); for_each_subsys(root, ss) - seq_printf(seq, ",%s", ss->name); + seq_show_option(seq, ss->name, NULL); if (test_bit(ROOT_NOPREFIX, &root->flags)) seq_puts(seq, ",noprefix"); if (strlen(root->release_agent_path)) - seq_printf(seq, ",release_agent=%s", root->release_agent_path); + seq_show_option(seq, "release_agent", + root->release_agent_path); if (clone_children(&root->top_cgroup)) seq_puts(seq, ",clone_children"); if (strlen(root->name)) - seq_printf(seq, ",name=%s", root->name); + seq_show_option(seq, "name", root->name); mutex_unlock(&cgroup_mutex); return 0; } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0cd7097a981a..c279f2f3372d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1011,7 +1011,7 @@ static void selinux_write_opts(struct seq_file *m, seq_puts(m, prefix); if (has_comma) seq_putc(m, '\"'); - seq_puts(m, opts->mnt_opts[i]); + seq_escape(m, opts->mnt_opts[i], "\"\n\\"); if (has_comma) seq_putc(m, '\"'); } |