Re: [PATCH] df: improve mount point selection with inaccurate mount list

[Bernhard Voelker]

On 08/18/2014 08:21 PM, Pádraig Brady wrote:
> v8.23 has a test failure on Fedora rawhide build servers
> in tests/df/skip-duplicate.sh.  This was due to no '/'
> entry being output by df.  That was due to an inaccurate
> /proc/mounts on the build environment as stat(/mnt/point)
> identified all these /proc/mounts entries as having the
> same device id:
>
>    /                    rootfs
>    /                    /dev/md1
>    /dev                 devtmpfs
>    /run                 tmpfs
>    /boot                /dev/md0
>    /proc/filesystems    /dev/md1
>
> Since the device name on the right changes for a given id,
> that causes the entries to be continually replaced, thus
> resulting in no '/' entry.  I'm guessing this is due to
> the mock environment bind mounting unneeded or sensitive
> items to a dummy file on the host / (/dev/md1) though
> have not looked into those details.

The same happened on openSUSE's OpenBuildService when the
package was built locally in a chroot: in this case, mtab
is wrong, too.

On the first glance, this could've been considered as a problem
in the test, i.e. a false assumption in tests/df/skip-duplicate.sh.

BTW: the device names do not necessarily have to be
trustworthy at all, because e.g. for 'proc' one could give
anything as device:

  mount -t proc /        /mnt/proc
  mount -t proc /hello   /mnt/proc
  mount -t proc /dev/sda /mnt/proc

> So rather than relying on an accurate /proc/mounts,
> the attached patch takes a more conservative replacement
> approach and only swaps a new device entry when the
> mount point matches.  That should handle all practical
> cases while also avoiding this situation.
>
> * src/df.c (filter_mount_list): Only replace entries with
> different device names when the mount point also matches.
> ---
>   src/df.c |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/df.c b/src/df.c
> index 3ef5d33..76aac7b 100644
> --- a/src/df.c
> +++ b/src/df.c
> @@ -646,7 +646,8 @@ filter_mount_list (bool devices_only)
>                     || (strlen (devlist->me->me_mountdir)
>                         > strlen (me->me_mountdir))
>                     /* or one overmounted on a different device.  */
> -                  || ! STREQ (devlist->me->me_devname, me->me_devname))
> +                  || (STREQ (me->me_mountdir, devlist->me->me_mountdir)
> +                      && ! STREQ (devlist->me->me_devname, me->me_devname)))
>                   {
>                     /* Discard mount entry for existing device.  */
>                     discard_me = devlist->me;

+1

Thanks & have a nice day,
Berny