[adding gnulib, coming from https://bugs.gnu.org/35137]
On 4/5/19 9:01 AM, Bernhard Voelker wrote: > On 4/4/19 9:52 AM, Zbigniew Jędrzejewski-Szmek wrote: >> See https://github.com/systemd/systemd/issues/12018 and >> https://github.com/karelzak/util-linux/issues/780 for additional context. >> >> $ mkdir "$(echo -e foo\\rbar)" >> $ sudo mount -t tmpfs tmpfs foo^Mbar/ >> $ cat -v /proc/self/mountinfo|grep foo >> 865 39 0:59 / /tmp/foo^Mbar rw,relatime shared:462 - tmpfs tmpfs rw,seclabel >> $ df -h | grep foo >> $ df -h /tmp/foo$'\r'bar >> Filesystem Size Used Avail Use% Mounted on >> - 3.9G 0 3.9G 0% /tmp/foo?bar >> >> When asked to show all filesystems, the mount point is not shown at all. >> When asked to show just that one, df parses the mount point correctly, >> but it gets the filesystem type wrong. > > Thanks for the report. > > I see the issue is not yet solved in util-linux as well. > For coreutils, the fix is in gnulib. Parsing the line is starting > to get ugly ... dirty patch attached. > > This also caters for the issue that df(1) totally skips a file system > if the source is an empty string which is allowed for e.g. tmpfs: > > $ mount -t tmpfs '' /mnt > $ df -h | grep mnt The attached is a patch for gnulib to fix the reported and two other issues when parsing /proc/self/mountinfo. Any objections? Meanwhile, I'm working a test for coreutils' df. FWIW: Karel fixed this issue in util-linux yesterday: https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/commit/?id=86673b3a46 Have a nice day, Berny
>From 15be8e6bcb6203fdb91d4a01fb65c2d4ef76995e Mon Sep 17 00:00:00 2001 From: Bernhard Voelker <m...@bernhard-voelker.de> Date: Tue, 9 Apr 2019 22:30:16 +0200 Subject: [PATCH] mountlist: make parsing /proc/self/mountinfo more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cater for the following issues with mountinfo parsing (the first one was reported by Zbigniew JÄdrzejewski-Szmek <zbys...@in.waw.pl> in <https://bugs.gnu.org/35137>). 1. The fields source, target, mntroot and fstype may contain characters like '\r'; sscanf(3) fails to read such values with the %s format specifier because it would stop at such characters. Example: "mount -t tmpfs tmpfs /foo^Mbar". The only true separator in that file is the ' ' character. 2. The source field may be an empty string, which happens e.g. with "mount -t tmpfs '' /target". 3. The fstype field may contain mangled characters as well which need unescaping. * lib/mountlist.c (terminate_at_blank): Add utility function. (read_file_system_list): In the block trying to read the mountinfo file, avoid using sscanf(3) with %s format; instead, parse the above fields separated by white spaces one by one. Handle the case when the source field is an empty string. Unescape the fstype field. --- ChangeLog | 22 +++++++++++++ lib/mountlist.c | 83 +++++++++++++++++++++++++++++++------------------ 2 files changed, 74 insertions(+), 31 deletions(-) diff --git a/ChangeLog b/ChangeLog index 87d95ed99..e049e81a0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2019-04-09 Bernhard Voelker <m...@bernhard-voelker.de> + + mountlist: make parsing /proc/self/mountinfo more robust + Cater for the following issues with mountinfo parsing (the first + one was reported by Zbigniew JÄdrzejewski-Szmek <zbys...@in.waw.pl> + in <https://bugs.gnu.org/35137>). + 1. The fields source, target, mntroot and fstype may contain characters + like '\r'; sscanf(3) fails to read such values with the %s format + specifier because it would stop at such characters. + Example: "mount -t tmpfs tmpfs /foo^Mbar". + The only true separator in that file is the ' ' character. + 2. The source field may be an empty string, which happens e.g. with + "mount -t tmpfs '' /target". + 3. The fstype field may contain mangled characters as well which need + unescaping. + * lib/mountlist.c (terminate_at_blank): Add utility function. + (read_file_system_list): In the block trying to read the mountinfo file, + avoid using sscanf(3) with %s format; instead, parse the above fields + separated by white spaces one by one. + Handle the case when the source field is an empty string. + Unescape the fstype field. + 2019-04-09 Bruno Haible <br...@clisp.org> openmp: Add workaround for 32-bit programs on AIX. diff --git a/lib/mountlist.c b/lib/mountlist.c index 9b54a2cf7..9e01d13f4 100644 --- a/lib/mountlist.c +++ b/lib/mountlist.c @@ -418,6 +418,18 @@ unescape_tab (char *str) str[j++] = str[i]; } } + +/* Find the next white space in STR, terminate the string there in place, + and return that position. Otherwise return NULL. */ + +static char * +terminate_at_blank (char const *str) +{ + char *s = NULL; + if ((s = strchr (str, ' ')) != NULL) + *s = '\0'; + return s; +} #endif /* Return a list of the currently mounted file systems, or NULL on error. @@ -453,56 +465,65 @@ read_file_system_list (bool need_fs_type) while (getline (&line, &buf_size, fp) != -1) { unsigned int devmaj, devmin; - int target_s, target_e, type_s, type_e; - int source_s, source_e, mntroot_s, mntroot_e; - char test; - char *dash; + int mntroot_s; + char *mntroot, *blank, *target, *dash, *fstype, *source; int rc; rc = sscanf(line, "%*u " /* id - discarded */ "%*u " /* parent - discarded */ "%u:%u " /* dev major:minor */ - "%n%*s%n " /* mountroot */ - "%n%*s%n" /* target, start and end */ - "%c", /* more data... */ + "%n", /* mountroot */ &devmaj, &devmin, - &mntroot_s, &mntroot_e, - &target_s, &target_e, - &test); + &mntroot_s); + + if (rc != 2 && rc != 3) /* 3 if %n included in count. */ + continue; - if (rc != 3 && rc != 7) /* 7 if %n included in count. */ + /* find end of MNTROOT. */ + mntroot = line + mntroot_s; + if (! (blank = terminate_at_blank (mntroot))) + continue; + + /* find end of TARGET. */ + target = blank + 1; + if (! (blank = terminate_at_blank (target))) continue; /* skip optional fields, terminated by " - " */ - dash = strstr (line + target_e, " - "); + dash = strstr (blank + 1, " - "); if (! dash) continue; - rc = sscanf(dash, " - " - "%n%*s%n " /* FS type, start and end */ - "%n%*s%n " /* source, start and end */ - "%c", /* more data... */ - &type_s, &type_e, - &source_s, &source_e, - &test); - if (rc != 1 && rc != 5) /* 5 if %n included in count. */ + /* advance past the " - " separator. */ + fstype = dash + 3; + if (! (blank = terminate_at_blank (fstype))) continue; + source = blank + 1; + if (*source == ' ') + { + /* The source is an empty string, which is e.g. allowed for + tmpfs: "mount -t tmpfs '' /mnt". */ + *source = '\0'; + } + else + { + if (! (blank = terminate_at_blank (source))) + continue; + } + /* manipulate the sub-strings in place. */ - line[mntroot_e] = '\0'; - line[target_e] = '\0'; - dash[type_e] = '\0'; - dash[source_e] = '\0'; - unescape_tab (dash + source_s); - unescape_tab (line + target_s); - unescape_tab (line + mntroot_s); + unescape_tab (source); + unescape_tab (target); + unescape_tab (mntroot); + unescape_tab (fstype); me = xmalloc (sizeof *me); - me->me_devname = xstrdup (dash + source_s); - me->me_mountdir = xstrdup (line + target_s); - me->me_mntroot = xstrdup (line + mntroot_s); - me->me_type = xstrdup (dash + type_s); + me->me_devname = xstrdup (source); + me->me_mountdir = xstrdup (target); + me->me_mntroot = xstrdup (mntroot); + me->me_type = xstrdup (fstype); me->me_type_malloced = 1; me->me_dev = makedev (devmaj, devmin); /* we pass "false" for the "Bind" option as that's only -- 2.21.0
