GNU bug report logs - #29038
df hangs on fifos/named pipes

Previous Next

Package: coreutils;

Reported by: Stephane Chazelas <stephane.chazelas <at> gmail.com>

Date: Sat, 28 Oct 2017 07:19:01 UTC

Severity: normal

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-coreutils <at> gnu.org:
bug#29038; Package coreutils. (Sat, 28 Oct 2017 07:19:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Stephane Chazelas <stephane.chazelas <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-coreutils <at> gnu.org. (Sat, 28 Oct 2017 07:19:02 GMT) Full text and rfc822 format available.

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

From: Stephane Chazelas <stephane.chazelas <at> gmail.com>
To: bug-coreutils <at> gnu.org
Cc: Martijn Dekker <martijn <at> inlv.org>
Subject: df hangs on fifos/named pipes
Date: Sat, 28 Oct 2017 08:18:11 +0100
test case:

   mkfifo p
   df p

That hangs, unless you make "p" non-readable or some other process
has the fifo open in write mode.

The reason is that df tries to open the fifo in read-only mode,
according to comments in the source code so as to trigger a
potential automout.

That goes back to this commit:

> commit dbd17157d7e693b8de9737f802db0e235ff5a3e6
> Author: Tomas Smetana <t.smetana <at> gmail.com>
> Date:   Tue Apr 28 11:21:49 2009 +0200
> 
>     df: use open(2), not stat, to trigger automounting
> 
>     * src/df.c (main): When iterating over command-line arguments,
>     attempting to ensure each backing file system is mounted, use
>     open, not stat.  stat is no longer sufficient to trigger
>     automounting, in some cases.  Based on a suggestion from Ian Kent.
>     More details in http://bugzilla.redhat.com/497830

More info at the bugzilla link.

It's arguable whether df, a reporting tool, should have such a
side effect as automounting a file system.

The fifo issue though is a bug IMO, especially considering that
POSIX explicitely says that df should work on fifos.

Here, it may be enough to add the O_DIRECTORY flag to open()
where available if we only care about automounting files of type
directory (or portably use opendir()).

Or use O_PATH  on Linux 3.6+ followed by openat() on non-fifos if
open(O_PATH) is not enough to trigger the automount in the
unlikely event we care about automounting non-directory files
(and report their disk usage).

Or not open() at all, and not automount file systems.

Note that busybox, heirloom or ast-open df implementations on
Linux don't have the problem (and presumably don't automount
file systems). Nor does FreeBSD.

Reproduced with:

$ df --version
df (GNU coreutils) 8.25

and:

$ df --version
df (GNU coreutils) 8.27.46-e13fe

That was discovered by Martijn Dekker, CCed, when looking for a
portable way to identify the file system of an arbitrary file.

-- 
Stephane




Information forwarded to bug-coreutils <at> gnu.org:
bug#29038; Package coreutils. (Sun, 29 Oct 2017 22:35:01 GMT) Full text and rfc822 format available.

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Stephane Chazelas <stephane.chazelas <at> gmail.com>, 29038 <at> debbugs.gnu.org
Cc: Martijn Dekker <martijn <at> inlv.org>
Subject: Re: bug#29038: df hangs on fifos/named pipes
Date: Sun, 29 Oct 2017 15:34:23 -0700
[Message part 1 (text/plain, inline)]
On 28/10/17 00:18, Stephane Chazelas wrote:
> test case:
> 
>    mkfifo p
>    df p
> 
> That hangs, unless you make "p" non-readable or some other process
> has the fifo open in write mode.
> 
> The reason is that df tries to open the fifo in read-only mode,
> according to comments in the source code so as to trigger a
> potential automout.
> 
> That goes back to this commit:
> 
>> commit dbd17157d7e693b8de9737f802db0e235ff5a3e6
>> Author: Tomas Smetana <t.smetana <at> gmail.com>
>> Date:   Tue Apr 28 11:21:49 2009 +0200
>>
>>     df: use open(2), not stat, to trigger automounting
>>
>>     * src/df.c (main): When iterating over command-line arguments,
>>     attempting to ensure each backing file system is mounted, use
>>     open, not stat.  stat is no longer sufficient to trigger
>>     automounting, in some cases.  Based on a suggestion from Ian Kent.
>>     More details in http://bugzilla.redhat.com/497830
> 
> More info at the bugzilla link.
> 
> It's arguable whether df, a reporting tool, should have such a
> side effect as automounting a file system.
> 
> The fifo issue though is a bug IMO, especially considering that
> POSIX explicitely says that df should work on fifos.
> 
> Here, it may be enough to add the O_DIRECTORY flag to open()
> where available if we only care about automounting files of type
> directory (or portably use opendir()).
> 
> Or use O_PATH  on Linux 3.6+ followed by openat() on non-fifos if
> open(O_PATH) is not enough to trigger the automount in the
> unlikely event we care about automounting non-directory files
> (and report their disk usage).
> 
> Or not open() at all, and not automount file systems.
> 
> Note that busybox, heirloom or ast-open df implementations on
> Linux don't have the problem (and presumably don't automount
> file systems). Nor does FreeBSD.
> 
> Reproduced with:
> 
> $ df --version
> df (GNU coreutils) 8.25
> 
> and:
> 
> $ df --version
> df (GNU coreutils) 8.27.46-e13fe
> 
> That was discovered by Martijn Dekker, CCed, when looking for a
> portable way to identify the file system of an arbitrary file.
> 

Yes we shouldn't hang.

RE side effects, open() is a fairly innocuous operation,
and we expect stat() to have side effects to auto mount.
(Note we avoid stat() with non specified arguments if possible due to this):
https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.27-92-ga19ff5d

I suppose we could stat() and if that succeeds && !fifo, only then call open() ?
Patch to do that is attached.

cheers,
Pádraig
[df-fifo-hang.patch (text/x-patch, attachment)]

Information forwarded to bug-coreutils <at> gnu.org:
bug#29038; Package coreutils. (Sun, 29 Oct 2017 23:23:01 GMT) Full text and rfc822 format available.

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

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 29038 <at> debbugs.gnu.org, Martijn Dekker <martijn <at> inlv.org>,
 Stephane Chazelas <stephane.chazelas <at> gmail.com>
Subject: Re: bug#29038: df hangs on fifos/named pipes
Date: Sun, 29 Oct 2017 16:21:52 -0700
On Sun, Oct 29, 2017 at 3:34 PM, Pádraig Brady <P <at> draigbrady.com> wrote:
...
>> That was discovered by Martijn Dekker, CCed, when looking for a
>> portable way to identify the file system of an arbitrary file.
>
> Yes we shouldn't hang.
>
> RE side effects, open() is a fairly innocuous operation,
> and we expect stat() to have side effects to auto mount.
> (Note we avoid stat() with non specified arguments if possible due to this):
> https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.27-92-ga19ff5d
>
> I suppose we could stat() and if that succeeds && !fifo, only then call open() ?
> Patch to do that is attached.

Sounds good. Patch looks fine to me. Thanks!




Information forwarded to bug-coreutils <at> gnu.org:
bug#29038; Package coreutils. (Mon, 30 Oct 2017 06:17:02 GMT) Full text and rfc822 format available.

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

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>,
 Stephane Chazelas <stephane.chazelas <at> gmail.com>, 29038 <at> debbugs.gnu.org
Cc: Martijn Dekker <martijn <at> inlv.org>
Subject: Re: bug#29038: df hangs on fifos/named pipes
Date: Sun, 29 Oct 2017 23:16:50 -0700
[Message part 1 (text/plain, inline)]
Pádraig Brady wrote:

> I suppose we could stat() and if that succeeds && !fifo, only then call open() ?
> Patch to do that is attached.

Better is to use open with O_NONBLOCK, as this avoids interpreting the file name 
twice in the usual case. Better yet is to use O_PATH if available, as this 
avoids interpreting the file name twice even when the file is unreadable or is a 
FIFO that would block. Also, the comment just before the code should be changed 
to match the altered code. Proposed followup patch attached.

It is puzzling that df calls fstat or stat, when it should just be calling 
fstatfs or statfs. But that is a different matter.
[0001-df-improve-fix-for-FIFOs.patch (text/x-patch, attachment)]

Information forwarded to bug-coreutils <at> gnu.org:
bug#29038; Package coreutils. (Mon, 30 Oct 2017 07:07:01 GMT) Full text and rfc822 format available.

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

From: Stephane Chazelas <stephane.chazelas <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Martijn Dekker <martijn <at> inlv.org>, 29038 <at> debbugs.gnu.org,
 Pádraig Brady <P <at> draigBrady.com>
Subject: Re: bug#29038: df hangs on fifos/named pipes
Date: Mon, 30 Oct 2017 07:06:10 +0000
2017-10-29 23:16:50 -0700, Paul Eggert:
> Pádraig Brady wrote:
> 
> >I suppose we could stat() and if that succeeds && !fifo, only then call open() ?
> >Patch to do that is attached.
> 
> Better is to use open with O_NONBLOCK, as this avoids interpreting the file
> name twice in the usual case.

That would still have the unwanted side effect of instantiating
(and kill thereafter) the pipe when opening a fifo that has a
writer and no reader.

As in, if some process does a fd = open("fifo", O_WRONLY)

The "df fifo" would cause that open() to return, and the next
write() to it possibly cause a SIGPIPE because df is already
gone.

It's different for O_DIRECTORY as the open() would fail on
fifos (it also fails if you don't have read permission,
which is another advantage of using O_PATH I suppose)


> Better yet is to use O_PATH if available, as
> this avoids interpreting the file name twice even when the file is
> unreadable or is a FIFO that would block.

Note that according to the man page, fstat() on fds open with
O_PATH only works since Linux 3.6. Note that we're already
interpreting the file path twice as we're not using fstatfs (as we
wouldn't want to cause a "too many open files" situation I
suppose by keeping all the files open before we get to the
second phase).

[...]
> It is puzzling that df calls fstat or stat, when it should just be calling
> fstatfs or statfs. But that is a different matter.

I suppose that's because it needs to determine if the file is a
device file or not as for device files where a fs is mounted, it
needs to report details for that fs as opposed to the fs the
device file is on.

-- 
Stephane




Reply sent to Pádraig Brady <P <at> draigBrady.com>:
You have taken responsibility. (Mon, 30 Oct 2017 16:54:02 GMT) Full text and rfc822 format available.

Notification sent to Stephane Chazelas <stephane.chazelas <at> gmail.com>:
bug acknowledged by developer. (Mon, 30 Oct 2017 16:54:02 GMT) Full text and rfc822 format available.

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>,
 Stephane Chazelas <stephane.chazelas <at> gmail.com>, 29038-done <at> debbugs.gnu.org
Cc: Martijn Dekker <martijn <at> inlv.org>
Subject: Re: bug#29038: df hangs on fifos/named pipes
Date: Mon, 30 Oct 2017 09:53:01 -0700
On 29/10/17 23:16, Paul Eggert wrote:
> Pádraig Brady wrote:
> 
>> I suppose we could stat() and if that succeeds && !fifo, only then call open() ?
>> Patch to do that is attached.
> 
> Better is to use open with O_NONBLOCK, as this avoids interpreting the file name 
> twice in the usual case. Better yet is to use O_PATH if available, as this 
> avoids interpreting the file name twice even when the file is unreadable or is a 
> FIFO that would block. Also, the comment just before the code should be changed 
> to match the altered code. Proposed followup patch attached.
> 
> It is puzzling that df calls fstat or stat, when it should just be calling 
> fstatfs or statfs. But that is a different matter.

I pushed my change with the comments fixed up.

I thought about O_NONBLOCK and O_PATH but thought these might not
induce or wait for the auto mount to occur.

thanks for the review,
Pádraig

marking this as done...





Information forwarded to bug-coreutils <at> gnu.org:
bug#29038; Package coreutils. (Mon, 30 Oct 2017 17:35:02 GMT) Full text and rfc822 format available.

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

From: Stephane Chazelas <stephane.chazelas <at> gmail.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 29038 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>,
 Martijn Dekker <martijn <at> inlv.org>
Subject: Re: bug#29038: df hangs on fifos/named pipes
Date: Mon, 30 Oct 2017 17:33:56 +0000
2017-10-30 09:53:01 -0700, Pádraig Brady:
[...]
> I pushed my change with the comments fixed up.
> 
> I thought about O_NONBLOCK and O_PATH but thought these might not
> induce or wait for the auto mount to occur.
[...]

Note that:

df /dev/watchdog

as root still causes the system to reboot on Linux. Not a POSIX
conformance issue as the behaviour is unspecified for device
files without a mounted file system on, but still not ideal.

The point is that opening files has or can have side effects and
df is not *meant* to open files.

I would think that automounting directories assuming we do want
it to happen should be enough unless someone can come up with a
realistic use case where non-directory files are being
automounted (and assuming that's possible).

-- 
Stephane




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 28 Nov 2017 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 6 years and 149 days ago.

Previous Next


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