df: output placeholder values for inaccessible mount points

A system provided mount entry may be unavailable due to TOCTOU race,
or if another device has been over-mounted at that position, or due to
access permissions.  In all these cases output "-" placeholder values
rather than either producing an error, or in the over-mount case
outputting values for the wrong device.

* src/df.c (device_list): A new global list now updated by
filter_mount_list().
(filter_mount_list): Adjust to take a parameter as to whether
update the global mount list, or only the mount <-> device ID mapping.
(get_dev): Use the device ID mapping to ensure we're not outputting
stats for the wrong device.  Also output placeholder values when we
can't access a system specified mount point.
(get_all_entries): Set the DEVICE_ONLY param for filter_mount_list().
(devname_for_dev): A new function to search the mount <-> dev mapping.
* test/df/skip-duplicates.sh: Adjust accordingly.
* NEWS: Mention the bug fixes.

Discussed at: http://bugs.gnu.org/16539

diff --git a/NEWS b/NEWS
index d3c23680c63ffcc9390a2fa0b2ae8c106d4bec9d..090254d8ac59c34381bf4ea28a573a757a5ec13c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -44,8 +44,10 @@ GNU coreutils NEWS                                    -*- outline -*-
 
   [These dd bugs were present in "the beginning".]
 
-  df now elides duplicates for virtual file systems like tmpfs, and will
-  display the correct device name for directories mounted multiple times.
+  df now elides duplicates for virtual file systems like tmpfs.
+  Displays the correct device details for points mounted multiple times.
+  Displays placeholder values for inaccessible file systems,
+  rather than error messages or values for the wrong file system.
   [These bugs were present in "the beginning".]
 
   du now silently ignores directory cycles introduced with bind mounts.

diff --git a/src/df.c b/src/df.c
index 10047cea11eec3b4484b8d7c8854eab0ace2438b..d6d4b0e9018c174dd825dd087a62dce789127af5 100644 (file)
--- a/src/df.c
+++ b/src/df.c
@@ -45,12 +45,12 @@
 
 /* Filled with device numbers of examined file systems to avoid
    duplicities in output.  */
-struct devlist
+static struct devlist
 {
   dev_t dev_num;
   struct mount_entry *me;
   struct devlist *next;
-};
+} *device_list;
 
 /* If true, show even file systems with zero size or
    uninteresting types.  */
@@ -607,23 +607,25 @@ excluded_fstype (const char *fstype)
    In the case of duplicities - based on the device number - the mount entry
    with a '/' in its me_devname (i.e. not pseudo name like tmpfs) wins.
    If both have a real devname (e.g. bind mounts), then that with the shorter
-   me_mountdir wins.  */
+   me_mountdir wins.  With DEVICES_ONLY == true (set with df -a), only update
+   the global device_list, rather than filtering the global mount_list.  */
 
 static void
-filter_mount_list (void)
+filter_mount_list (bool devices_only)
 {
   struct mount_entry *me;
 
-  /* Store of already-processed device numbers.  */
-  struct devlist *devlist_head = NULL;
-
-  /* Sort all 'wanted' entries into the list devlist_head.  */
+  /* Sort all 'wanted' entries into the list device_list.  */
   for (me = mount_list; me;)
     {
       struct stat buf;
       struct devlist *devlist;
       struct mount_entry *discard_me = NULL;
 
+      /* TODO: On Linux we might avoid this stat() and another in get_dev()
+         by using the device IDs available from /proc/self/mountinfo.
+         read_file_system_list() could populate me_dev from those
+         for efficiency and accuracy.  */
       if (-1 == stat (me->me_mountdir, &buf))
         {
           /* Stat failed - add ME to be able to complain about it later.  */
@@ -632,7 +634,7 @@ filter_mount_list (void)
       else
         {
           /* If we've already seen this device...  */
-          for (devlist = devlist_head; devlist; devlist = devlist->next)
+          for (devlist = device_list; devlist; devlist = devlist->next)
             if (devlist->dev_num == buf.st_dev)
               break;
 
@@ -661,8 +663,9 @@ filter_mount_list (void)
 
       if (discard_me)
         {
-           me = me->me_next;
-           free_mount_entry (discard_me);
+          me = me->me_next;
+          if (! devices_only)
+            free_mount_entry (discard_me);
         }
       else
         {
@@ -670,26 +673,46 @@ filter_mount_list (void)
           devlist = xmalloc (sizeof *devlist);
           devlist->me = me;
           devlist->dev_num = buf.st_dev;
-          devlist->next = devlist_head;
-          devlist_head = devlist;
+          devlist->next = device_list;
+          device_list = devlist;
 
           me = me->me_next;
         }
     }
 
   /* Finally rebuild the mount_list from the devlist.  */
-  mount_list = NULL;
-  while (devlist_head)
+  if (! devices_only) {
+    mount_list = NULL;
+    while (device_list)
+      {
+        /* Add the mount entry.  */
+        me = device_list->me;
+        me->me_next = mount_list;
+        mount_list = me;
+        /* Free devlist entry and advance.  */
+        struct devlist *devlist = device_list->next;
+        free (device_list);
+        device_list = devlist;
+      }
+  }
+}
+
+/* Search a mount entry list for device id DEV.
+   Return the corresponding device name if found or NULL if not.  */
+
+static char const * _GL_ATTRIBUTE_PURE
+devname_for_dev (dev_t dev)
+{
+  struct devlist *dl = device_list;
+
+  while (dl)
     {
-      /* Add the mount entry.  */
-      me = devlist_head->me;
-      me->me_next = mount_list;
-      mount_list = me;
-      /* Free devlist entry and advance.  */
-      struct devlist *devlist = devlist_head->next;
-      free (devlist_head);
-      devlist_head = devlist;
+      if (dl->dev_num == dev)
+        return dl->me->me_devname;
+      dl = dl->next;
     }
+
+  return NULL;
 }
 
 /* Return true if N is a known integer value.  On many file systems,
@@ -878,9 +901,40 @@ get_dev (char const *disk, char const *mount_point, char const* file,
     fsu = *force_fsu;
   else if (get_fs_usage (stat_file, disk, &fsu))
     {
-      error (0, errno, "%s", quote (stat_file));
-      exit_status = EXIT_FAILURE;
-      return;
+      /* If we can't access a system provided entry due
+         to it not being present (now), or due to permissions,
+         just output placeholder values rather than failing.  */
+      if (process_all && (errno == EACCES || errno == ENOENT))
+        {
+          if (! show_all_fs)
+            return;
+
+          fstype = "-";
+          fsu.fsu_blocksize = fsu.fsu_blocks = fsu.fsu_bfree =
+          fsu.fsu_bavail = fsu.fsu_files = fsu.fsu_ffree = UINTMAX_MAX;
+        }
+      else
+        {
+          error (0, errno, "%s", quote (stat_file));
+          exit_status = EXIT_FAILURE;
+          return;
+        }
+    }
+  else if (process_all && show_all_fs)
+    {
+      /* Ensure we don't output incorrect stats for over-mounted directories.
+         Discard stats when the device name doesn't match.  */
+      struct stat sb;
+      if (stat (stat_file, &sb) == 0)
+        {
+          char const * devname = devname_for_dev (sb.st_dev);
+          if (devname && ! STREQ (devname, disk))
+            {
+              fstype = "-";
+              fsu.fsu_blocksize = fsu.fsu_blocks = fsu.fsu_bfree =
+              fsu.fsu_bavail = fsu.fsu_files = fsu.fsu_ffree = UINTMAX_MAX;
+            }
+        }
     }
 
   if (fsu.fsu_blocks == 0 && !show_all_fs && !show_listed_fs)
@@ -1230,8 +1284,7 @@ get_all_entries (void)
 {
   struct mount_entry *me;
 
-  if (!show_all_fs)
-    filter_mount_list ();
+  filter_mount_list (show_all_fs);
 
   for (me = mount_list; me; me = me->me_next)
     get_dev (me->me_devname, me->me_mountdir, NULL, NULL, me->me_type,

diff --git a/tests/df/skip-duplicates.sh b/tests/df/skip-duplicates.sh
index a620e7320c0a3c1620d2a982b7a521f761c26e51..9f0f749fef97aa6f884a40818cb5a94c21d51d98 100755 (executable)
--- a/tests/df/skip-duplicates.sh
+++ b/tests/df/skip-duplicates.sh
@@ -96,8 +96,8 @@ test -f x || skip_ "internal test failure: maybe LD_PRELOAD doesn't work?"
 LD_PRELOAD=./k.so df -T >out || fail=1
 test $(wc -l <out) -eq $(expr 1 + $unique_entries) || { fail=1; cat out; }
 
-# Ensure we fail when unable to stat invalid entries
-LD_PRELOAD=./k.so CU_TEST_DUPE_INVALID=1 df -T >out && fail=1
+# Ensure we don't fail when unable to stat (currently) unavailable entries
+LD_PRELOAD=./k.so CU_TEST_DUPE_INVALID=1 df -T >out || fail=1
 test $(wc -l <out) -eq $(expr 1 + $unique_entries) || { fail=1; cat out; }
 
 # df should also prefer "/fsname" over "fsname"
@@ -113,6 +113,8 @@ test $(grep -c 'virtfs2.*fstype2' <out) -eq 1 || { fail=1; cat out; }
 # Ensure that filtering duplicates does not affect -a processing.
 LD_PRELOAD=./k.so df -a >out || fail=1
 test $(wc -l <out) -eq 6 || { fail=1; cat out; }
+# Ensure placeholder "-" values used for the eclipsed "virtfs"
+test $(grep -c 'virtfs *-' <out) -eq 1 || { fail=1; cat out; }
 
 # Ensure that filtering duplicates does not affect
 # argument processing (now without the fake getmntent()).