GNU bug report logs - #21372
df prioritizes some bind mounts over real ones.

Previous Next

Full log

Message #32 received at 21372 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: dave.chiluk <at> canonical.com, Dave Chiluk <chiluk <at> canonical.com>, 
 21372 <at> debbugs.gnu.org
Subject: Re: bug#21372: [PATCH] df: fix prioritize real mounts over bind mounts
Date: Fri, 11 Sep 2015 23:02:33 +0100

On 11/09/15 22:55, Dave Chiluk wrote:
> On 09/11/2015 03:42 PM, Pádraig Brady wrote:
>> On 31/08/15 22:05, Dave Chiluk wrote:
>>> Fixes an issue where bind mounts with shorter mount directories than the
>>> original mount are prioritized when running df.  The root cause of this
>>> is that /proc/self/mountinfo now lists the filesystem device with bind
>>> mounts rather than the source directory. With /etc/mtab the source
>>> device was listed as the originating directory so this was not an issue.
>>>
>>> More information is available here.
>>> https://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/1432871
>>> ---
>>>  src/df.c | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/df.c b/src/df.c
>>> index 2e541b9..00c77c1 100644
>>> --- a/src/df.c
>>> +++ b/src/df.c
>>> @@ -652,9 +652,14 @@ filter_mount_list (bool devices_only)
>>>                else if ((strchr (me->me_devname, '/')
>>>                         /* let "real" devices with '/' in the name win.  */
>>>                          && ! strchr (devlist->me->me_devname, '/'))
>>> -                       /* let a shorter mountdir win.  */
>>> -                       || (strlen (devlist->me->me_mountdir)
>>> +                       /* let a shorter mountdir win. */
>>> +                       /* Only if it's not a bind mount. */
>>> +                       || ((strlen (devlist->me->me_mountdir)
>>>                             > strlen (me->me_mountdir))
>>> +                          && (devlist->me->me_mntroot != NULL
>>> +                             && me->me_mntroot != NULL
>>> +                             && (strlen (devlist->me->me_mntroot)
>>> +                                 > strlen(me->me_mntroot))))
>>
>> As previously mentioned, should this be >= ?
> Sorry my apologies for not addressing that.  I don't think it should be
> (strlen (devlist->me->me_mntroot)>= strlen(me->me_mntroot)), because
> that would give preference to later mounts of the same me_mntroot
> directory *(I'm specifically thinking of the case where both me_mntroot
> = "/").
> 
> However, looking at the code again, I realized that by adding the NULL
> checks I have actually changed the behavior for non-linux OS's since the
> NULL checks would always defeat the (strlen (devlist->me->me_mountdir) >
> strlen (me->me_mountdir) check.
> 
> I'm going to think through this nasty if case and see if I can make it
> prettier.
> 
>>
>> We'll need to look at tests.
>> This will be quite tricky given the current
>> avoidance of /proc/self/mountinfo in tests.
>> I'll look at adding a separate test for this.
>>
>> thanks,
>> Pádraig.
>>
> 
> Additionally I agree with you that we should mirror the find_mnt
> bracketting behavior.
>     /dev/sdb2       xfs              12.7G  9.4G   3.3G  74% /
>     /dev/sdb2[/etc] xfs                                     /root/chroot
> 
> I'll submit that in a different patch once we have this more squared away.


Well I mentioned that df was "DEVICE oriented" and so should
probably not worry about distinguishing that, especially as
the functionality is already provided by findmnt(1).

thanks,
Pádraig

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.