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>
[ luis: backported to 3.16:
  - dropped changes to fs/overlayfs/super.c, net/ceph/ceph_common.c and to
    cgroup_show_options() in kernel/cgroup.c
  - adjusted context ]
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 8503ec130c1574324bff343799514e0657a7b89c..526db4502aa625697bc5df3d61f29b6d42b70db6 100644 (file)
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -483,7 +483,8 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
        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 88839806742007ca8dfaf8d77754695d4924fef9..d678cda1cd817a3144ad0278454be63812fd5c7f 100644 (file)
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -382,17 +382,17 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
        struct sockaddr *srcaddr;
        srcaddr = (struct sockaddr *)&tcon->ses->server->srcaddr;
 
-       seq_printf(s, ",vers=%s", tcon->ses->server->vals->version_string);
+       seq_show_option(s, "vers", tcon->ses->server->vals->version_string);
        cifs_show_security(s, tcon->ses);
        cifs_show_cache_flavor(s, cifs_sb);
 
        if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER)
                seq_puts(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 cf977e2f8a6ffe14ee5a69136c58453bba8fdb3e..a8f6f6a358ce85642f1e3b400aa467b883e6f144 100644 (file)
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1750,10 +1750,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]);
 #endif
 }
 

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 1319b5c4ec68bea0da503caf6e170e0cb196f6fb..b844cf82e0f92ca50d918cdc6284a3da0b4fe7b8 100644 (file)
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1302,11 +1302,11 @@ static int gfs2_show_options(struct seq_file *s, struct dentry *root)
        if (is_ancestor(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 eee7206c38d18e1a3d3b1697fd429763e3fe9f5b..410b65eea683dda33ec0324888dc38707d1850f1 100644 (file)
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -135,9 +135,9 @@ static int hfs_show_options(struct seq_file *seq, struct dentry *root)
        struct hfs_sb_info *sbi = HFS_SB(root->d_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",
                        from_kuid_munged(&init_user_ns, sbi->s_uid),
                        from_kgid_munged(&init_user_ns, sbi->s_gid));

diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c
index c90b72ee676d8a022dd47b8577be6f6a40967425..bb806e58c9770ec5491235bb8ac5fcdcd2e5574b 100644 (file)
--- a/fs/hfsplus/options.c
+++ b/fs/hfsplus/options.c
@@ -218,9 +218,9 @@ int hfsplus_show_options(struct seq_file *seq, struct dentry *root)
        struct hfsplus_sb_info *sbi = HFSPLUS_SB(root->d_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,
                        from_kuid_munged(&init_user_ns, sbi->uid),
                        from_kgid_munged(&init_user_ns, sbi->gid));

diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
index bb529f3b7f2bf8a119ec5f2eba3c260e09c361c7..456f60e2552eba1f8ff8b629da9ee8b7e5804006 100644 (file)
--- a/fs/hostfs/hostfs_kern.c
+++ b/fs/hostfs/hostfs_kern.c
@@ -255,7 +255,7 @@ static int hostfs_show_options(struct seq_file *seq, struct dentry *root)
        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 ddb662b32447ca49cd206c3f4e1394ee558ceeb3..34690360ff333c570a5c31e0b7365f000da9419d 100644 (file)
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1564,8 +1564,8 @@ static int ocfs2_show_options(struct seq_file *s, struct dentry *root)
                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/reiserfs/super.c b/fs/reiserfs/super.c
index 27575041bea3408f217edbfd51f665519988526c..48c6924eba05bf3ec10b1b72ed89f888f10d0d27 100644 (file)
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -710,18 +710,20 @@ static int reiserfs_show_options(struct seq_file *seq, struct dentry *root)
                seq_puts(seq, ",acl");
 
        if (REISERFS_SB(s)->s_jdev)
-               seq_printf(seq, ",jdev=%s", REISERFS_SB(s)->s_jdev);
+               seq_show_option(seq, "jdev", REISERFS_SB(s)->s_jdev);
 
        if (journal->j_max_commit_age != journal->j_default_max_commit_age)
                seq_printf(seq, ",commit=%d", journal->j_max_commit_age);
 
 #ifdef CONFIG_QUOTA
        if (REISERFS_SB(s)->s_qf_names[USRQUOTA])
-               seq_printf(seq, ",usrjquota=%s", REISERFS_SB(s)->s_qf_names[USRQUOTA]);
+               seq_show_option(seq, "usrjquota",
+                               REISERFS_SB(s)->s_qf_names[USRQUOTA]);
        else if (opts & (1 << REISERFS_USRQUOTA))
                seq_puts(seq, ",usrquota");
        if (REISERFS_SB(s)->s_qf_names[GRPQUOTA])
-               seq_printf(seq, ",grpjquota=%s", REISERFS_SB(s)->s_qf_names[GRPQUOTA]);
+               seq_show_option(seq, "grpjquota",
+                               REISERFS_SB(s)->s_qf_names[GRPQUOTA]);
        else if (opts & (1 << REISERFS_GRPQUOTA))
                seq_puts(seq, ",grpquota");
        if (REISERFS_SB(s)->s_jquota_fmt) {

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 8f0333b3f7a011a25960c3ace80b57ebcaf58a19..2637010a62245a6b0db8697b52bcd612157e3da1 100644 (file)
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -529,9 +529,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 52e0097f61f00108f0faec9b37c57c2f03e1fcb2..6288341db62d1aca7b4b70f04b19a734684f9c56 100644 (file)
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -158,6 +158,41 @@ static inline struct user_namespace *seq_user_ns(struct seq_file *seq)
 #endif
 }
 
+/**
+ * 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)
 /*
  * Helpers for iteration over list_head-s in seq_files

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 44fadfb287bffd7492f6b6dd6885ed6510ed8ffe..58fd665d12175486bb0cfe97932372d2972b16cd 100644 (file)
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1248,13 +1248,14 @@ static int cgroup_show_options(struct seq_file *seq,
 
        spin_lock(&release_agent_path_lock);
        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);
        spin_unlock(&release_agent_path_lock);
 
        if (test_bit(CGRP_CPUSET_CLONE_CHILDREN, &root->cgrp.flags))
                seq_puts(seq, ",clone_children");
        if (strlen(root->name))
-               seq_printf(seq, ",name=%s", root->name);
+               seq_show_option(seq, "name", root->name);
        return 0;
 }
 

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9a23a7cccdc4215d8c2c4f7a49b0b15ac0aa7534..af2d5124f99ccbc50cb5ee752d83ec9691d0792a 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1100,7 +1100,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, '\"');
        }