Skip Menu |
Report information
Id: 123443
Status: resolved
Priority: 0/
Queue: perl5

Owner: tonyc <tony [at] develop-help.com>
Requestors: slaven [at] rezic.de
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: 5.22.0

Attachments
0001-perl-123443-avoid-overflowing-got-into-a-negative-nu.patch
0002-don-t-allow-a-negative-file-position-on-a-PerlIO-sca.patch
io-scalar-segfault.pl



Subject: Very large seek in a read-only scalar variable filehandle segfaults
Download (untitled) / with headers
text/plain 1.2k
The attached script is causing a segmentation fault on FreeBSD and Linux systems with newer perls (5.16 .. 5.21) or may consume lots of memory with older perls (5.8 .. 5.14). The script is basically doing a huge seek() with a subsequent read(). Doing the same with a read-only file is not causing any problems, because Unix does not extend the file and treats the huge seek position as an eof position. Also it seems that the CPAN module IO::Scalar does not have any problems. valgrind says the following: ==32540== Process terminating with default action of signal 11 (SIGSEGV) ==32540== Access not within mapped region at address 0x1062CE2A0 ==32540== at 0x4C2A760: memcpy (mc_replace_strmem.c:838) ==32540== by 0x689E1B8: PerlIOScalar_read (in /bbbike/perl-5.20.1/lib/5.20.1/x86_64-linux/auto/PerlIO/scalar/scalar.so) ==32540== by 0x4E8F39: Perl_pp_sysread (in /bbbike/perl-5.20.1/bin/perl) ==32540== by 0x4A4E32: Perl_runops_standard (in /bbbike/perl-5.20.1/bin/perl) ==32540== by 0x43FFD3: perl_run (in /bbbike/perl-5.20.1/bin/perl) ==32540== by 0x422284: main (in /bbbike/perl-5.20.1/bin/perl) The original problem was reported for the CPAN module Image::Info: https://rt.cpan.org/Ticket/Display.html?id=100847 Regards, Slaven
Subject: io-scalar-segfault.pl
#!/usr/bin/perl use strict; use Fcntl qw(SEEK_SET); use Test::More 'no_plan'; sub run_seek_and_read ($) { my $fh = shift; seek $fh, 2**32, SEEK_SET or die $!; read $fh, my $buf, 1; ok eof($fh), 'eof check'; } { open my $fh, "<", $0 or die $!; run_seek_and_read $fh; pass 'seek in a readonly file'; } if (eval { require IO::Scalar; 1 }) { my $buf0 = "hello"; my $fh = IO::Scalar->new(\$buf0); run_seek_and_read $fh; pass 'seek with IO::Scalar'; } if (0 && eval { require IO::String; 1 }) { # consumes much memory and may also segfault my $buf0 = "hello"; my $fh = IO::String->new($buf0); run_seek_and_read $fh; pass 'seek with IO::Scalar'; } { my $buf0 = "hello"; open my $fh, "<", \$buf0 or die $!; run_seek_and_read $fh; # segfaults pass 'seek in a readonly scalar ref'; } __END__
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.4k
On Tue Dec 16 13:00:59 2014, slaven@rezic.de wrote: Show quoted text
> The attached script is causing a segmentation fault on FreeBSD and > Linux systems with newer perls (5.16 .. 5.21) or may consume lots of > memory with older perls (5.8 .. 5.14). The script is basically doing a > huge seek() with a subsequent read(). Doing the same with a read-only > file is not causing any problems, because Unix does not extend the > file and treats the huge seek position as an eof position. Also it > seems that the CPAN module IO::Scalar does not have any problems. > > valgrind says the following: > > ==32540== Process terminating with default action of signal 11 > (SIGSEGV) > ==32540== Access not within mapped region at address 0x1062CE2A0 > ==32540== at 0x4C2A760: memcpy (mc_replace_strmem.c:838) > ==32540== by 0x689E1B8: PerlIOScalar_read (in /bbbike/perl- > 5.20.1/lib/5.20.1/x86_64-linux/auto/PerlIO/scalar/scalar.so) > ==32540== by 0x4E8F39: Perl_pp_sysread (in /bbbike/perl- > 5.20.1/bin/perl) > ==32540== by 0x4A4E32: Perl_runops_standard (in /bbbike/perl- > 5.20.1/bin/perl) > ==32540== by 0x43FFD3: perl_run (in /bbbike/perl-5.20.1/bin/perl) > ==32540== by 0x422284: main (in /bbbike/perl-5.20.1/bin/perl) > > The original problem was reported for the CPAN module Image::Info: > https://rt.cpan.org/Ticket/Display.html?id=100847
Please try the attached patches. The first fixes your test cases, the second prevents the stored file position from being negative. Tony
Subject: 0001-perl-123443-avoid-overflowing-got-into-a-negative-nu.patch
From 8800bfd6ddc89d521140c034b3c07646e6e8da7c Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Wed, 17 Dec 2014 13:32:43 +1100 Subject: [perl #123443] avoid overflowing got into a negative number --- ext/PerlIO-scalar/scalar.pm | 2 +- ext/PerlIO-scalar/scalar.xs | 12 +++++++++--- ext/PerlIO-scalar/t/scalar.t | 11 ++++++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/ext/PerlIO-scalar/scalar.pm b/ext/PerlIO-scalar/scalar.pm index 2dca6b0..03f60b2 100644 --- a/ext/PerlIO-scalar/scalar.pm +++ b/ext/PerlIO-scalar/scalar.pm @@ -1,5 +1,5 @@ package PerlIO::scalar; -our $VERSION = '0.20'; +our $VERSION = '0.21'; require XSLoader; XSLoader::load(); 1; diff --git a/ext/PerlIO-scalar/scalar.xs b/ext/PerlIO-scalar/scalar.xs index 67e9ae3..f130c0c 100644 --- a/ext/PerlIO-scalar/scalar.xs +++ b/ext/PerlIO-scalar/scalar.xs @@ -152,7 +152,7 @@ PerlIOScalar_read(pTHX_ PerlIO *f, void *vbuf, Size_t count) SV *sv = s->var; char *p; STRLEN len; - I32 got; + STRLEN got; p = SvPV(sv, len); if (SvUTF8(sv)) { if (sv_utf8_downgrade(sv, TRUE)) { @@ -165,9 +165,15 @@ PerlIOScalar_read(pTHX_ PerlIO *f, void *vbuf, Size_t count) return -1; } } - got = len - (STRLEN)(s->posn); - if (got <= 0) + /* I assume that Off_t is at least as large as len (which + * seems safe) and that the size of the buffer in our SV is + * always less than half the size of the address space + */ + assert(sizeof(Off_t) >= sizeof(len)); + assert((Off_t)len >= 0); + if ((Off_t)len <= s->posn) return 0; + got = len - (STRLEN)(s->posn); if ((STRLEN)got > (STRLEN)count) got = (STRLEN)count; Copy(p + (STRLEN)(s->posn), vbuf, got, STDCHAR); diff --git a/ext/PerlIO-scalar/t/scalar.t b/ext/PerlIO-scalar/t/scalar.t index 9bc1abe..547ecea 100644 --- a/ext/PerlIO-scalar/t/scalar.t +++ b/ext/PerlIO-scalar/t/scalar.t @@ -16,7 +16,7 @@ use Fcntl qw(SEEK_SET SEEK_CUR SEEK_END); # Not 0, 1, 2 everywhere. $| = 1; -use Test::More tests => 114; +use Test::More tests => 118; my $fh; my $var = "aaa\n"; @@ -491,3 +491,12 @@ my $byte_warning = "Strings with code points over 0xFF may not be mapped into in print $refh "boo\n"; is $x, $as_string."boo\n", 'string gets appended to ref'; } + +{ # [perl #123443] + my $buf0 = "hello"; + open my $fh, "<", \$buf0 or die $!; + ok(seek($fh, 2**32, SEEK_SET), "seek to a large position"); + is(read($fh, my $tmp, 1), 0, "read from a large offset"); + is($tmp, "", "should have read nothing"); + ok(eof($fh), "fh should be eof"); +} -- 1.7.10.4
Subject: 0002-don-t-allow-a-negative-file-position-on-a-PerlIO-sca.patch
From 56967151c5b8559b443a0b5fbb8d7331e3277d20 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Wed, 17 Dec 2014 13:15:53 +1100 Subject: don't allow a negative file position on a PerlIO::scalar handle previosly seek() would produce an error, but would still make the\ file position negative. --- ext/PerlIO-scalar/scalar.xs | 10 ++++++---- ext/PerlIO-scalar/t/scalar.t | 9 ++++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/ext/PerlIO-scalar/scalar.xs b/ext/PerlIO-scalar/scalar.xs index f130c0c..9e9f7c2 100644 --- a/ext/PerlIO-scalar/scalar.xs +++ b/ext/PerlIO-scalar/scalar.xs @@ -103,28 +103,30 @@ IV PerlIOScalar_seek(pTHX_ PerlIO * f, Off_t offset, int whence) { PerlIOScalar *s = PerlIOSelf(f, PerlIOScalar); + Off_t new_posn = -1; switch (whence) { case SEEK_SET: - s->posn = offset; + new_posn = offset; break; case SEEK_CUR: - s->posn = offset + s->posn; + new_posn = offset + s->posn; break; case SEEK_END: { STRLEN oldcur; (void)SvPV(s->var, oldcur); - s->posn = offset + oldcur; + new_posn = offset + oldcur; break; } } - if (s->posn < 0) { + if (new_posn < 0) { if (ckWARN(WARN_LAYER)) Perl_warner(aTHX_ packWARN(WARN_LAYER), "Offset outside string"); SETERRNO(EINVAL, SS_IVCHAN); return -1; } + s->posn = new_posn; return 0; } diff --git a/ext/PerlIO-scalar/t/scalar.t b/ext/PerlIO-scalar/t/scalar.t index 547ecea..f1156d6 100644 --- a/ext/PerlIO-scalar/t/scalar.t +++ b/ext/PerlIO-scalar/t/scalar.t @@ -16,7 +16,7 @@ use Fcntl qw(SEEK_SET SEEK_CUR SEEK_END); # Not 0, 1, 2 everywhere. $| = 1; -use Test::More tests => 118; +use Test::More tests => 120; my $fh; my $var = "aaa\n"; @@ -500,3 +500,10 @@ my $byte_warning = "Strings with code points over 0xFF may not be mapped into in is($tmp, "", "should have read nothing"); ok(eof($fh), "fh should be eof"); } + +{ + my $buf0 = "hello"; + open my $fh, "<", \$buf0 or die $!; + ok(!seek($fh, -10, SEEK_CUR), "seek to negative position"); + is(tell($fh), 0, "shouldn't change the position"); +} -- 1.7.10.4
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
Dana Uto 16. Pro 2014, 18:50:47, tonyc reče: Show quoted text
> On Tue Dec 16 13:00:59 2014, slaven@rezic.de wrote:
> > The attached script is causing a segmentation fault on FreeBSD and > > Linux systems with newer perls (5.16 .. 5.21) or may consume lots of > > memory with older perls (5.8 .. 5.14). The script is basically doing a > > huge seek() with a subsequent read(). Doing the same with a read-only > > file is not causing any problems, because Unix does not extend the > > file and treats the huge seek position as an eof position. Also it > > seems that the CPAN module IO::Scalar does not have any problems. > > > > valgrind says the following: > > > > ==32540== Process terminating with default action of signal 11 > > (SIGSEGV) > > ==32540== Access not within mapped region at address 0x1062CE2A0 > > ==32540== at 0x4C2A760: memcpy (mc_replace_strmem.c:838) > > ==32540== by 0x689E1B8: PerlIOScalar_read (in /bbbike/perl- > > 5.20.1/lib/5.20.1/x86_64-linux/auto/PerlIO/scalar/scalar.so) > > ==32540== by 0x4E8F39: Perl_pp_sysread (in /bbbike/perl- > > 5.20.1/bin/perl) > > ==32540== by 0x4A4E32: Perl_runops_standard (in /bbbike/perl- > > 5.20.1/bin/perl) > > ==32540== by 0x43FFD3: perl_run (in /bbbike/perl-5.20.1/bin/perl) > > ==32540== by 0x422284: main (in /bbbike/perl-5.20.1/bin/perl) > > > > The original problem was reported for the CPAN module Image::Info: > > https://rt.cpan.org/Ticket/Display.html?id=100847
> > Please try the attached patches. > > The first fixes your test cases, the second prevents the stored file > position from being negative.
Thanks, seems to work fine. The segfault from the CPAN RT 100847 test case does not happen with this patch. Regards, Slaven
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.9k
On Tue Dec 16 22:56:43 2014, slaven@rezic.de wrote: Show quoted text
> Dana Uto 16. Pro 2014, 18:50:47, tonyc reče:
> > On Tue Dec 16 13:00:59 2014, slaven@rezic.de wrote:
> > > The attached script is causing a segmentation fault on FreeBSD and > > > Linux systems with newer perls (5.16 .. 5.21) or may consume lots > > > of > > > memory with older perls (5.8 .. 5.14). The script is basically > > > doing a > > > huge seek() with a subsequent read(). Doing the same with a read- > > > only > > > file is not causing any problems, because Unix does not extend the > > > file and treats the huge seek position as an eof position. Also it > > > seems that the CPAN module IO::Scalar does not have any problems. > > > > > > valgrind says the following: > > > > > > ==32540== Process terminating with default action of signal 11 > > > (SIGSEGV) > > > ==32540== Access not within mapped region at address 0x1062CE2A0 > > > ==32540== at 0x4C2A760: memcpy (mc_replace_strmem.c:838) > > > ==32540== by 0x689E1B8: PerlIOScalar_read (in /bbbike/perl- > > > 5.20.1/lib/5.20.1/x86_64-linux/auto/PerlIO/scalar/scalar.so) > > > ==32540== by 0x4E8F39: Perl_pp_sysread (in /bbbike/perl- > > > 5.20.1/bin/perl) > > > ==32540== by 0x4A4E32: Perl_runops_standard (in /bbbike/perl- > > > 5.20.1/bin/perl) > > > ==32540== by 0x43FFD3: perl_run (in /bbbike/perl- > > > 5.20.1/bin/perl) > > > ==32540== by 0x422284: main (in /bbbike/perl-5.20.1/bin/perl) > > > > > > The original problem was reported for the CPAN module Image::Info: > > > https://rt.cpan.org/Ticket/Display.html?id=100847
> > > > Please try the attached patches. > > > > The first fixes your test cases, the second prevents the stored file > > position from being negative.
> > Thanks, seems to work fine. The segfault from the CPAN RT 100847 test > case does not happen with this patch.
Thanks. I've applied the first patch as 63d073d27fe50d823f0e3c528ac62c9aa584704d and a variant of the second as 1d050e5534ce798acb8f9cd9c56c9f557ec658e0. Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 439b
On Wed Dec 17 15:08:14 2014, tonyc wrote: Show quoted text
> I've applied the first patch as > 63d073d27fe50d823f0e3c528ac62c9aa584704d and a variant of the second > as 1d050e5534ce798acb8f9cd9c56c9f557ec658e0.
Even after v5.21.6-601-gaa67537, the Windows smokes are still failing. See <nntp://nntp.perl.org/20141219145203.7AAA34E01CC@zwei> for instance. Is there any chance you could look into it before tomorrow’s release? -- Father Chrysostomos
Subject: Re: [perl #123443] Very large seek in a read-only scalar variable filehandle segfaults
From: Tony Cook <tony [...] develop-help.com>
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Date: Sat, 20 Dec 2014 13:14:42 +1100
CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 579b
On Fri, Dec 19, 2014 at 07:57:03AM -0800, Father Chrysostomos via RT wrote: Show quoted text
> On Wed Dec 17 15:08:14 2014, tonyc wrote:
> > I've applied the first patch as > > 63d073d27fe50d823f0e3c528ac62c9aa584704d and a variant of the second > > as 1d050e5534ce798acb8f9cd9c56c9f557ec658e0.
> > Even after v5.21.6-601-gaa67537, the Windows smokes are still failing. See <nntp://nntp.perl.org/20141219145203.7AAA34E01CC@zwei> for instance. > > Is there any chance you could look into it before tomorrow’s release?
Strange, I tested it on 32-bit windows. Trying again with blead. Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 775b
On Fri Dec 19 18:15:11 2014, tonyc wrote: Show quoted text
> On Fri, Dec 19, 2014 at 07:57:03AM -0800, Father Chrysostomos via RT > wrote:
> > On Wed Dec 17 15:08:14 2014, tonyc wrote:
> > > I've applied the first patch as > > > 63d073d27fe50d823f0e3c528ac62c9aa584704d and a variant of the > > > second > > > as 1d050e5534ce798acb8f9cd9c56c9f557ec658e0.
> > > > Even after v5.21.6-601-gaa67537, the Windows smokes are still > > failing. See <nntp://nntp.perl.org/20141219145203.7AAA34E01CC@zwei> > > for instance. > > > > Is there any chance you could look into it before tomorrow’s release?
> > Strange, I tested it on 32-bit windows. > > Trying again with blead.
Looks like the smokes build with a 32-bit Off_t. Fixed in 9745959a89f3a201c789b8e4ce494405f95b2a7a, I hope. Tony Tony
Subject: Your ticket against Perl 5 has been resolved
Download (untitled) / with headers
text/plain 263b
Thanks for submitting this ticket The issue should be resolved with the release today of Perl v5.22, available at http://www.perl.org/get.html If you find that the problem persists, feel free to reopen this ticket -- Karl Williamson for the Perl 5 porters team


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org