Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #1111459 > unrolled thread

Bug#1016369: IO::Handle ->error does not work, always saying "fine"

Back to article view | Back to linux.debian.bugs.dist

Contents

  Bug#1016369: IO::Handle ->error does not work, always saying "fine" Ian Jackson <ijackson@chiark.greenend.org.uk> - 2022-07-30 14:50 +0200
    Bug#1016369: IO::Handle ->error does not work, always saying "fine" Niko Tyni <ntyni@debian.org> - 2022-08-06 20:10 +0200
      Bug#1016369: IO::Handle ->error does not work, always saying "fine" Ian Jackson <ijackson@chiark.greenend.org.uk> - 2022-08-06 21:50 +0200
        Bug#1016369: IO::Handle ->error does not work, always saying "fine" Niko Tyni <ntyni@debian.org> - 2022-08-07 20:10 +0200

#1111459 — Bug#1016369: IO::Handle ->error does not work, always saying "fine"

Package: perl
Version: 5.34.0-4
Severity: grave

To reproduce

    perl -MIO::Handle -e 'open X, "<", "." or die $!; $_ = <X>; printf "%s %s %s\n", X->error(), $!;'
    perl -MIO::Handle -e 'open X, ">", "/dev/full" or die $!; print X 1; flush X; printf "%s %s %s\n", X->error(), $!; close X'

Expected output

    -1 Bad file descriptor
    -1 No space left on device

Actual output

    0 Bad file descriptor
    0 No space left on device

This is quite alarming.  I think it makes it in fact impossible to
read files fully reliably in Perl.  "use autodie" does not seem to help.

And scripts might reasonably have expected that they could defer error
handling by testing error() rather than each call (as one can in C).

I think this used to work, but, evidently, only in the distant past,
since my jessie chroot doesn't get this right either.

Justification for the severity:

Can cause data loss: if a file is opened but unreadable for any
reason, the program will process the part (if any) that will is
readable and then 

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.

[toc] | [next] | [standalone]

#1112271

Control: severity -1 normal

Hi, thanks Ian for the report and Damyan for looking into the issues.

On Sun, Jul 31, 2022 at 11:37:09AM +0300, Damyan Ivanov wrote:
> -=| Ian Jackson, 30.07.2022 13:42:05 +0100 |=-
> > Package: perl
> > Version: 5.34.0-4
> > Severity: grave
> > 
> > To reproduce
> > 
> >     perl -MIO::Handle -e 'open X, "<", "." or die $!; $_ = <X>; printf "%s %s %s\n", X->error(), $!;'
> >     perl -MIO::Handle -e 'open X, ">", "/dev/full" or die $!; print X 1; flush X; printf "%s %s %s\n", X->error(), $!; close X'
> > 
> > Expected output
> > 
> >     -1 Bad file descriptor
> >     -1 No space left on device
> > 
> > Actual output
> > 
> >     0 Bad file descriptor
> >     0 No space left on device


FWIW I get 

    0 Is a directory 
    1 No space left on device 

on sid (perl_5.34.0-5). I'm not sure why you'd expect -1. The documentation
for IO::Handle::error() only mentions it reporting a true value.

The first issue (reading a directory as a plain file) seems to be about
the error flag getting cleared when reading past EOF or something like
that. According to my meager debugger skills, the clearing happens around

  https://sources.debian.org/src/perl/5.34.0-5/pp_hot.c/#L3278

As Damyan notes, you can detect this by checking for EOF before reading,
although

   perl -MIO::Handle -e 'open X, "<", "." or die $!; $_ = <X> if !X->eof(); printf "%s %s\n", X->error(), $!;'
   1 Inappropriate ioctl for device 

is not quite what I'd expect either.

Anyway, the data loss argument seems misplaced given we've read all the
data there is when we are at EOF?

I can see there is probably a bug around here, but it would be good
to have an example test case that doesn't involve treating a directory
as a plain file (which can have very varying results between different
platforms) before taking this upstream. Particularly so if you want to
argue it's as serious as claimed here.

See also https://github.com/Perl/perl5/issues/12782 which has not
received attention in almost ten years.


The second issue (writing to /dev/full) is indeed fixed in sid / Perl
5.34.  It was https://github.com/Perl/perl5/issues/6799 and reportedly
only affects things like character devices (including /dev/full) and
sockets. I've verified that trying to write to a normal file on a full
filesystem does set the error() flag on stable / Perl 5.32.

I think that makes the issue less severe, and I'm not very inclined to
fix it in stable.  But in case we end up doing that anyway, these would
be the commits needed:

    https://github.com/Perl/perl5/commit/89341f87f9fc65c4d7133e497bb04586e86b8052

    https://github.com/Perl/perl5/commit/8a2562bec7cd9f8eff6812f340f99dddd028bb33

Downgrading the severity, but let me know what you think based on the above.
-- 
Niko

[toc] | [prev] | [next] | [standalone]

#1112284

Niko Tyni writes ("Re: Bug#1016369: IO::Handle ->error does not work, always saying "fine""):
> Hi, thanks Ian for the report and Damyan for looking into the issues.

Indeed, thanks to you and to Damyan.
> > > Actual output
> > > 
> > >     0 Bad file descriptor
> > >     0 No space left on device
> 
> 
> FWIW I get 
> 
>     0 Is a directory 
>     1 No space left on device 
> 
> on sid (perl_5.34.0-5). I'm not sure why you'd expect -1. The documentation
> for IO::Handle::error() only mentions it reporting a true value.

Interesting.  I forget the details (can't easily check now) but some
test that passed for me gave -1.  I agree that 1 is better.

> The first issue (reading a directory as a plain file) seems to be about
> the error flag getting cleared when reading past EOF or something like
> that.

I used reading from a directory as an example for two reason:

Firstly, it was the situation that actually happened to me.  I was
writing a program and it had a bug that caused it to erroneously read
a directory, rather than some file within it.  The program became
convinced the "file" was empty, leading to strange malfunctions.  I
would have expected the file reading error checks ought to have
detected this earlier.

Secondly, having discovered this bug, reading from a directory is one
of very few ways to get something you can open() for reading, which
then returns errors if you call read(2).

Another way to obtain this erroneous behaviour is to pass a perl
script a stdin which is not, in fact, open for reading (easily
achieved with shell redirection).  However, I didn't use that for my
bug report (even though it does repro the bug) because STDIN seems
like it will have special magic.

I would love a more portable name for something which can be opened
for reading, but which can't then be read.

I think the reasons so few people have reported this bug before are as
follows:

1. This can only occur if you have something open for reading but
which turns out not to be readable.  In the absence of strange bugs,
that usually means a hardware failure resulting in EIO, which is very
rare.

2. If and when someone does get data loss due to this bug in a
situation where they got EIO due to hardware failure, they are very
likely to (a) be preoccupied with the disaster that is no doubt
unfolding (b) unable to precisely reproduce the situation or indeed
have precise and accurate records.
    
3. Very few people are so careful about error handling anyway, sadly.

So IMO the fact that this bug has been rarely reported does not mean
that it isn't a data loss bug.  Rather, it's a data loss bug which
mostly has the effect of burning up more things when your system is
already on fire.

> Anyway, the data loss argument seems misplaced given we've read all the
> data there is when we are at EOF?

No.

The handle in the test case can never be successfully read, so it has
*not* reached EOF.  All the calls to read(2) return errors.  If perl
claims that the handle has reached EOF, that is itself a bug.  (A data
loss bug, because a fslse EOF causes the rest of the data to be
ignored.)

I am almost certain that this bug would trip if you got EIO while
trying to read a plain file.  (But this is hard to test.)

> See also https://github.com/Perl/perl5/issues/12782 which has not
> received attention in almost ten years.

Yikes.  That does seem like the very same bug.

> The second issue (writing to /dev/full) is indeed fixed in sid / Perl
> 5.34.  It was https://github.com/Perl/perl5/issues/6799 and reportedly
> only affects things like character devices (including /dev/full) and
> sockets. I've verified that trying to write to a normal file on a full
> filesystem does set the error() flag on stable / Perl 5.32.

I wonder what is different about plain files.  I suspect the fact that
it "works" (correctly returning errors) for you in this case may be
due to luck (the precise series of calls).

> I think that makes the issue less severe, and I'm not very inclined to
> fix it in stable.  But in case we end up doing that anyway, these would
> be the commits needed:
> 
>     https://github.com/Perl/perl5/commit/89341f87f9fc65c4d7133e497bb04586e86b8052
>     https://github.com/Perl/perl5/commit/8a2562bec7cd9f8eff6812f340f99dddd028bb33
> 
> Downgrading the severity, but let me know what you think based on the above.

I still think this is a data loss bug.  (Two bugs.)

I will think about trying to produce more convincing repros.  Would
you convinced by something involving an LD_PRELOAD to cause "syscalls"
to fail ?  Something involving ptrace ?

Damyan writes:
> Note that the recommended way to read files line by line is (perldoc 
> -f readline):
> 
>     while ( ! eof($fh) ) {
>         defined( $_ = readline $fh ) or die "readline failed: $!";
>         ...
>     }

I don't find this particularly convincing.  This argument seems to be
saying "never use <> to read lines" which is pretty strange.  Surely
it should be possible to use "<>" in its line-reading mode, without
data loss.  (And, with autodie, without having to do an explicit error
check.)  The behaviour of ->error() seems contrary to the
documentation, and the "use <> and then check ->error()" idiom seems
to me to be both justifiable by the text, and reasonable.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.

[toc] | [prev] | [next] | [standalone]

#1112358

On Sat, Aug 06, 2022 at 08:44:18PM +0100, Ian Jackson wrote:

> > The second issue (writing to /dev/full) is indeed fixed in sid / Perl
> > 5.34.  It was https://github.com/Perl/perl5/issues/6799 and reportedly
> > only affects things like character devices (including /dev/full) and
> > sockets. I've verified that trying to write to a normal file on a full
> > filesystem does set the error() flag on stable / Perl 5.32.
> 
> I wonder what is different about plain files.  I suspect the fact that
> it "works" (correctly returning errors) for you in this case may be
> due to luck (the precise series of calls).

You might want to read the bug then, particularly Tony Cook's comment at

  https://github.com/Perl/perl5/issues/6799#issuecomment-626511037

Quoting:

    Perl's I/O objects (that is the IO SV , not PerlIO) can have both an
    input (PerlIO) file handle and an output file handle. For files that
    output handle is the same as the input handle [...] For a character
    device (like /dev/full) or a socket, perl creates a second PerlIO
    object on the same fd to use as the output handle, and that causes
    the behaviour this ticket documents.

So I don't think luck has anything to do with it.

> > Downgrading the severity, but let me know what you think based on the above.
> 
> I still think this is a data loss bug.  (Two bugs.)

I think it's at most one but whatever.

I've just reported https://github.com/Perl/perl5/issues/20060 about the
read errors. You might want to do your convincing there if needed :)
Obviously it's not going to be fixed on Debian side without upstream
involvement, whatever the severity.

> >     while ( ! eof($fh) ) {
> >         defined( $_ = readline $fh ) or die "readline failed: $!";
> >         ...
> >     }
> 
> I don't find this particularly convincing.  This argument seems to be
> saying "never use <> to read lines" which is pretty strange.  Surely
> it should be possible to use "<>" in its line-reading mode, without
> data loss.

Yeah, I agree.
-- 
Niko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.bugs.dist

csiph-web