GNU bug report logs - #50012
[PATCH] df: Fixed usage of incorrect stat data when using automount

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 50012 in the body.
You can then email your comments to 50012 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Thimo Emmerich <thimo.emmerich <at> sony.com>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] df: Fixed usage of incorrect stat data when using automount
Date: Wed, 11 Aug 2021 17:59:29 +0200

[Message part 1 (text/plain, inline)]

This seems to be a regression from b04ce61958c1: "df: fix hang with fifo 
argument".
When using df on an browsable automount map (in my case NFS mounts) df 
returns a screwed up information if the mount is not existing.
During execution it triggers the mount via open() but it uses previously 
collected stat information for the output.
A second run leads to the correct information due to the mount in the 
first run.
Example:
me <at> app07:/psg$ ls -ld psg01bu
drwxr-xr-x 2 root root 0 Jul 23 17:33 psg01bu
me <at> app07:/psg$ df -h psg01bu
Filesystem      Size  Used Avail Use% Mounted on
auto_psg        2.0T   80M  1.9T   1% /psg
me <at> app07:/psg$ ls -ld psg01bu
drwxrwsr-x 2 root abc 4096 May  5 11:37 psg01bu
me <at> app07:/psg$ df -h psg01bu
Filesystem               Size  Used Avail Use% Mounted on
fil36:/vg1/lv06/psg01bu  2.0T   80M  1.9T   1% /psg/psg01bu

This behaviour could be observed on Ubuntu 20.04's stock df command 
(coreutils 8.30-3ubuntu2) as well as on latest GIT master. An old CentOS 
7 machine is not affected.

In general the comment in df.h "stat each of the given entries to make 
sure any corresponding partition is automounted." is suspicious since in 
[1] it is explicitly stated that "Any access to this directory beyond a 
“stat” will (normally) cause the d_op->d_automount() dentry operation to 
be called.".

Since the stat in [2]/df.c, line 1775 is necessary for the following 
fifo evaluation, it seems to be necessary to update the stat information 
after a successful open.

Best regards,
 Thimo

[1]: https://www.kernel.org/doc/html/latest/filesystems/autofs.html
[2]: https://github.com/coreutils/coreutils/blob/master/src/df.c#L1775

From a586ab553d9e6f463049349a8ca88d7e727804df Mon Sep 17 00:00:00 2001
From: Thimo Emmerich <thimo.emmerich <at> sony.com>
Date: Wed, 11 Aug 2021 17:50:22 +0200
Subject: [PATCH] df: Fixed usage of incorrect stat data when using automount

This code re-runs stat() after open()ing to update possibly
invalid data. Invalid data can occur in case of using df on
an unmounted directory in a browsable automount map.
---
 src/df.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/df.c b/src/df.c
index 4534935f5..a5288cb6a 100644
--- a/src/df.c
+++ b/src/df.c
@@ -1766,9 +1766,10 @@ main (int argc, char **argv)

   if (optind < argc)
     {
-      /* stat each of the given entries to make sure any corresponding
-         partition is automounted.  This must be done before reading the
-         file system table.  */
+      /* get the stat() information for the fifo check.
+         In case of a browsable automount map this stat will
+         not trigger the mount. Thus the information needs
+         to be updated.  */
       stats = xnmalloc (argc - optind, sizeof *stats);
       for (int i = optind; i < argc; ++i)
         {
@@ -1780,10 +1781,15 @@ main (int argc, char **argv)
             }
           else if (! S_ISFIFO (stats[i - optind].st_mode))
             {
-              /* open() is needed to automount in some cases.  */
+                /* open() each of the given entries to make sure any 
corresponding
+                 partition is automounted.  This must be done before 
reading the
+                 file system table.  */
               int fd = open (argv[i], O_RDONLY | O_NOCTTY);
               if (0 <= fd)
-                close (fd);
+                {
+                  close (fd);
+                  stat (argv[i], &stats[i - optind]);
+                }
             }
         }
     }
--
2.25.1

-- 
Mit freundlichen Grüßen / with kind regards

  Thimo Emmerich

-------------------------------------------

R&D Center Europe Stuttgart Laboratory 01

Phone:    +49 (0) 711 / 5858 - 282
E-mail:   thimo.emmerich <at> eu.sony.com

Sony Europe B.V., Zweigniederlassung Deutschland
Stuttgart Technology Center
Hedelfinger Str. 61, 70327 Stuttgart, Germany

Registered at the Court of Berlin-Charlottenburg HRB 129332 B

Main Establishment: Sony Europe B.V.
Registered Office: The Heights, Brooklands, Weybridge, Surrey, KT13 0XW, UK
Incorporated in the Netherlands No. 71682147
Directors: Hideyuki Furumi, Masaki Kurebayashi, Ricky Londema

[Message part 2 (text/html, inline)]

Message #10 received at 50012-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Thimo Emmerich <thimo.emmerich <at> sony.com>
Cc: 50012-done <at> debbugs.gnu.org
Subject: Re: bug#50012: [PATCH] df: Fixed usage of incorrect stat data when
 using automount
Date: Wed, 11 Aug 2021 11:24:03 -0700

[Message part 1 (text/plain, inline)]

On 8/11/21 8:59 AM, Thimo Emmerich wrote:

> Since the stat in [2]/df.c, line 1775 is necessary for the following 
> fifo evaluation, it seems to be necessary to update the stat information 
> after a successful open.

Thanks for reporting the bug. I installed the attached. It differs a bit 
from what you proposed in that it prefers fstat to stat, as fstat should 
be a bit faster and should avoid some unlikely races (there are many 
more races, but one fix at a time).

[0001-df-fix-bug-with-automounted.patch (text/x-patch, attachment)]

Message #13 received at 50012-done <at> debbugs.gnu.org (full text, mbox):

From: Thimo Emmerich <thimo.emmerich <at> sony.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 50012-done <at> debbugs.gnu.org
Subject: Re: bug#50012: [PATCH] df: Fixed usage of incorrect stat data when
 using automount
Date: Thu, 12 Aug 2021 09:16:23 +0200

Thank you for taking care. Your fix seems more elaborated and works for me.

On 11.08.21 20:24, Paul Eggert wrote:
> On 8/11/21 8:59 AM, Thimo Emmerich wrote:
>
>> Since the stat in [2]/df.c, line 1775 is necessary for the following 
>> fifo evaluation, it seems to be necessary to update the stat 
>> information after a successful open.
>
> Thanks for reporting the bug. I installed the attached. It differs a 
> bit from what you proposed in that it prefers fstat to stat, as fstat 
> should be a bit faster and should avoid some unlikely races (there are 
> many more races, but one fix at a time).

Previous Next

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