GNU bug report logs -
#29038
df hangs on fifos/named pipes
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 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.
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):
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):
[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):
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):
[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):
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):
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):
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 7 years and 162 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.