linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] proc: Escape more characters in /proc/mounts output
@ 2020-12-15 12:53 Siddhesh Poyarekar
  2020-12-16  4:33 ` Al Viro
  0 siblings, 1 reply; 3+ messages in thread
From: Siddhesh Poyarekar @ 2020-12-15 12:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, Alexander Viro, Florian Weimer

When a filesystem is mounted with a blank name like so:

 # mount '' bad -t tmpfs

its name entry in /proc/mounts is blank causing the line to start
with a space.

 /mnt/bad tmpfs rw,seclabel,relatime,inode64 0 0

Further, the name could start with a hash, causing the entry to look
like this (leading space added so that git does not strip it out):

 # /mnt/bad tmpfs rw,seclabel,relatime,inode64 0 0

This breaks getmntent and any code that aims to parse fstab as well as
/proc/mounts with the same logic since they need to strip leading
spaces or skip over comments, due to which they report incorrect
output or skip over the line respectively.

This fix resolves both issues by (1) treating blank names the same way
as not having a name and (2) by escaping the hash character into its
octal encoding, which getmntent can then decode and print correctly.
As far as file parsing is concerned, these are the only additional
cases to cater for since they cover all characters that have a special
meaning in that context.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
Cc: Florian Weimer <fweimer@redhat.com>
---
Changes from V1:
- Break out a separate routine copy_mount_devname for device name copy.
- Rename copy_mount_string to copy_mount_type since it is now single
  use.

 fs/namespace.c      | 36 +++++++++++++++++++++++++++++++++---
 fs/proc_namespace.c |  2 +-
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index cebaa3e81794..01fafcbd9f5b 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3108,11 +3108,41 @@ static void *copy_mount_options(const void __user * data)
 	return copy;
 }
 
-static char *copy_mount_string(const void __user *data)
+static char *copy_mount_type(const void __user *data)
 {
 	return data ? strndup_user(data, PATH_MAX) : NULL;
 }
 
+static char *copy_mount_devname(const void __user *data)
+{
+	char *p;
+	long length;
+
+	if (data == NULL)
+		return NULL;
+
+	length = strnlen_user(data, PATH_MAX);
+
+	if (!length)
+		return ERR_PTR(-EFAULT);
+
+	if (length > PATH_MAX)
+		return ERR_PTR(-EINVAL);
+
+	/* Ignore blank strings */
+	if (length == 1)
+		return NULL;
+
+	p = memdup_user(data, length);
+
+	if (IS_ERR(p))
+		return p;
+
+	p[length - 1] = '\0';
+
+	return p;
+}
+
 /*
  * Flags is a 32-bit value that allows up to 31 non-fs dependent flags to
  * be given to the mount() call (ie: read-only, no-dev, no-suid etc).
@@ -3408,12 +3438,12 @@ SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
 	char *kernel_dev;
 	void *options;
 
-	kernel_type = copy_mount_string(type);
+	kernel_type = copy_mount_type(type);
 	ret = PTR_ERR(kernel_type);
 	if (IS_ERR(kernel_type))
 		goto out_type;
 
-	kernel_dev = copy_mount_string(dev_name);
+	kernel_dev = copy_mount_devname(dev_name);
 	ret = PTR_ERR(kernel_dev);
 	if (IS_ERR(kernel_dev))
 		goto out_dev;
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index e59d4bb3a89e..090b53120b7a 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -83,7 +83,7 @@ static void show_mnt_opts(struct seq_file *m, struct vfsmount *mnt)
 
 static inline void mangle(struct seq_file *m, const char *s)
 {
-	seq_escape(m, s, " \t\n\\");
+	seq_escape(m, s, " \t\n\\#");
 }
 
 static void show_type(struct seq_file *m, struct super_block *sb)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] proc: Escape more characters in /proc/mounts output
  2020-12-15 12:53 [PATCH v2] proc: Escape more characters in /proc/mounts output Siddhesh Poyarekar
@ 2020-12-16  4:33 ` Al Viro
  2020-12-16  5:25   ` Siddhesh Poyarekar
  0 siblings, 1 reply; 3+ messages in thread
From: Al Viro @ 2020-12-16  4:33 UTC (permalink / raw)
  To: Siddhesh Poyarekar; +Cc: linux-kernel, linux-fsdevel, Florian Weimer

On Tue, Dec 15, 2020 at 06:23:18PM +0530, Siddhesh Poyarekar wrote:

> +static char *copy_mount_devname(const void __user *data)
> +{
> +	char *p;
> +	long length;
> +
> +	if (data == NULL)
> +		return NULL;
> +
> +	length = strnlen_user(data, PATH_MAX);
> +
> +	if (!length)
> +		return ERR_PTR(-EFAULT);
> +
> +	if (length > PATH_MAX)
> +		return ERR_PTR(-EINVAL);
> +
> +	/* Ignore blank strings */
> +	if (length == 1)
> +		return NULL;
> +
> +	p = memdup_user(data, length);

Once more, with feeling: why bother?  What's wrong
with using the damn strndup_user() and then doing
whatever checks you want with the data already
copied, living in normal kernel memory, with all
string functions applicable, etc.?

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] proc: Escape more characters in /proc/mounts output
  2020-12-16  4:33 ` Al Viro
@ 2020-12-16  5:25   ` Siddhesh Poyarekar
  0 siblings, 0 replies; 3+ messages in thread
From: Siddhesh Poyarekar @ 2020-12-16  5:25 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, linux-fsdevel, Florian Weimer

On 12/16/20 10:03 AM, Al Viro wrote:
> Once more, with feeling: why bother?  What's wrong
> with using the damn strndup_user() and then doing
> whatever checks you want with the data already
> copied, living in normal kernel memory, with all
> string functions applicable, etc.?

I was trying to avoid the allocation, but I reckon it is pointless to 
micro-optimize the invalid case.  I'll send v3.

Siddhesh

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-16  5:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 12:53 [PATCH v2] proc: Escape more characters in /proc/mounts output Siddhesh Poyarekar
2020-12-16  4:33 ` Al Viro
2020-12-16  5:25   ` Siddhesh Poyarekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).