>From 0a4b8027049f6746a237c9fc34a0e0a4afdcfc62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= Date: Wed, 4 Jun 2014 00:09:11 +0100 Subject: [PATCH] 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 overmounted at that position, or due to access permissions. In all these cases output "-" placeholder values rather than either producing an error, or in the overmount case outputting values for the wrong device. * src/df.c (device_list): A new global list updated from ... (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 plaeholder values when we can't access a system specified mount point. (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 --- NEWS | 6 ++- src/df.c | 106 ++++++++++++++++++++++++++++++++----------- tests/df/skip-duplicates.sh | 9 +++- 3 files changed, 89 insertions(+), 32 deletions(-) diff --git a/NEWS b/NEWS index 5d1fe99..7ba145e 100644 --- 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".] head --bytes=-N and --lines=-N now handles devices more diff --git a/src/df.c b/src/df.c index 10047ce..c7da208 100644 --- 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. */ @@ -610,20 +610,21 @@ excluded_fstype (const char *fstype) me_mountdir wins. */ 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 +633,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 +662,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 +672,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 +900,37 @@ 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)) + { + 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 overmounted 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 +1280,10 @@ get_all_entries (void) { struct mount_entry *me; - if (!show_all_fs) - filter_mount_list (); + if (! show_all_fs) + filter_mount_list (false); + else + filter_mount_list (true); 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 a620e73..1b19149 100755 --- a/tests/df/skip-duplicates.sh +++ b/tests/df/skip-duplicates.sh @@ -96,9 +96,10 @@ 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 && fail=1 -test $(wc -l out || fail=1 +test $(wc -l out || fail=1 test $(wc -l