| Message ID | 20201215042454.998361-1-siddhesh@gotplt.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | proc: Escape more characters in /proc/mounts output | expand |
On Tue, Dec 15, 2020 at 09:54:54AM +0530, Siddhesh Poyarekar wrote: > + get_user(byte, (const char __user *)data); > + > + return byte ? strndup_user(data, PATH_MAX) : NULL; > } No. Not to mention anything else, you * fetch the same data twice * fail to check the get_user() results
On 12/15/20 11:40 AM, Al Viro wrote: > On Tue, Dec 15, 2020 at 09:54:54AM +0530, Siddhesh Poyarekar wrote: > >> + get_user(byte, (const char __user *)data); >> + >> + return byte ? strndup_user(data, PATH_MAX) : NULL; >> } > > No. Not to mention anything else, you > * fetch the same data twice > * fail to check the get_user() results > Ahh sorry, I could inline the strndup_user call and put an additional check for length == 1 to return NULL. Would that be acceptable? The other alternative would be to not touch copy_mount_string and instead, check after fetching the string and if it is blank, free it and set to NULL. That seems more expensive though. Siddhesh
diff --git a/fs/namespace.c b/fs/namespace.c index cebaa3e81794..68bd5a814a2a 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -3110,7 +3110,13 @@ static void *copy_mount_options(const void __user * data) static char *copy_mount_string(const void __user *data) { - return data ? strndup_user(data, PATH_MAX) : NULL; + char byte; + if (data == NULL) + return NULL; + + get_user(byte, (const char __user *)data); + + return byte ? strndup_user(data, PATH_MAX) : NULL; } /* 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)
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> --- fs/namespace.c | 8 +++++++- fs/proc_namespace.c | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-)