Segmentation fault in Perl_pp_multiply (and other functions)

p5pRT commented 9 years ago

Migrated from rt.perl.org#126042 (status was 'resolved')

Searchable as RT126042$

p5pRT commented 9 years ago

From @dcollinsn

Greetings Porters\,

I have compiled bleadperl with the afl-gcc compiler using:

./Configure -Dusedevel -Dprefix='/usr/local/perl-afl' -Dcc=afl-gcc -Duselongdouble -Duse64bitint -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -des AFL_HARDEN=1 make && make test

And then fuzzed the resulting binary using:

AFL_NO_VAR_CHECK=1 afl-fuzz -i in -o out bin/perl @@

After reducing testcases using `afl-tmin` and filtering out testcases that are merely iterations of "#!perl -u"\, I have located the following testcase that triggers a segmentation fault in the perl interpreter. The testcase is the 15-byte file:

&{0==0}*&{0==0}

Several other variations of this failed\, but this is the simplest example. All have either two calls to "&{0==0}" or nested calls of that type "&{0==&{0==0}}". This looks vaguely similar to the bugs arising from the stack not being refcounted (c.f. 125840\, 123710\, 123804\, 123997\, but the failure point is different. Unfortunately\, no valgrind is available\, as I haven't yet had a chance to upgrade to a version of valgrind that's aware of dwarf 4 and I'm off to work. If you for whatever reason are unable to repro this\, I'll try to add updated information early this week.

**GDB**

GNU gdb (GDB) 7.10 Copyright (C) 2015 Free Software Foundation\, Inc. License GPLv3+: GNU GPL version 3 or later \<http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY\, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "i686-pc-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions\, please see: \<http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: \<http://www.gnu.org/software/gdb/documentation/>. For help\, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from ../bin/perl...done. (gdb) run Starting program: /usr/local/perl-afl/bin/perl f4i000000 [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/libthread_db.so.1".

Program received signal SIGSEGV\, Segmentation fault. 0x083ff152 in Perl_pp_multiply () at pp.c:1275 1275 tryAMAGICbin_MG(mult_amg\, AMGf_assign|AMGf_numeric); (gdb) bt #0 0x083ff152 in Perl_pp_multiply () at pp.c:1275 #1 0x083506fb in Perl_runops_standard () at run.c:41 #2 0x08116c47 in S_run_body (oldscope=1) at perl.c:2456 #3 perl_run (my_perl=0x87d8008) at perl.c:2379 #4 0x08068762 in main (argc=2\, argv=0xbffff7e4\, env=0xbffff7f0) at perlmain.c:116

**PERL -V** Summary of my perl5 (revision 5 version 23 subversion 3) configuration: Commit id: 718ac8a2e5fd26fa68078f057e7c332594ebebf6 Platform: osname=linux\, osvers=2.6.32-5-686\, archname=i686-linux-64int-ld uname='linux nagios 2.6.32-5-686 #1 smp tue may 13 16:33:32 utc 2014 i686 gnulinux ' config_args='-Dusedevel -Dprefix=/usr/local/perl-afl -Dcc=afl-gcc -Duselongdouble -Duse64bitint -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -des' hint=recommended\, useposix=true\, d_sigaction=define useithreads=undef\, usemultiplicity=undef use64bitint=define\, use64bitall=undef\, uselongdouble=define usemymalloc=n\, bincompat5005=undef Compiler: cc='afl-gcc'\, ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\, optimize='-g'\, cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion=''\, gccversion='5.2.0'\, gccosandvers='' intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=12345678\, doublekind=3 d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=12\, longdblkind=3 ivtype='long long'\, ivsize=8\, nvtype='long double'\, nvsize=12\, Off_t='off_t'\, lseeksize=8 alignbytes=4\, prototype=define Linker and Libraries: ld='afl-gcc'\, ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/local/lib/gcc/i686-pc-linux-gnu/5.2.0/include-fixed /usr/lib /lib/../lib /usr/lib/../lib /lib /usr/lib/i486-linux-gnu /usr/lib64 libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.22.so\, so=so\, useshrplib=false\, libperl=libperl.a gnulibc_version='2.22' Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E' cccdlflags='-fPIC'\, lddlflags='-shared -g -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_COPY_ON_WRITE PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_LONG_DOUBLE USE_PERLIO USE_PERL_ATOF Built under linux Compiled at Sep 8 2015 22:24:21 @INC: /usr/local/perl-afl/lib/site_perl/5.23.3/i686-linux-64int-ld /usr/local/perl-afl/lib/site_perl/5.23.3 /usr/local/perl-afl/lib/5.23.3/i686-linux-64int-ld /usr/local/perl-afl/lib/5.23.3 /usr/local/perl-afl/lib/site_perl/5.23.2 /usr/local/perl-afl/lib/site_perl .

p5pRT commented 9 years ago

From @dcollinsn

%% VALGRIND OUTPUT FOR CANONICAL TESTCASE ABOVE %%

==6140== Memcheck\, a memory error detector ==6140== Copyright (C) 2002-2013\, and GNU GPL'd\, by Julian Seward et al. ==6140== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info ==6140== Command: ../bin/perl f4i000000 ==6140== ==6140== Invalid read of size 4 ==6140== at 0x83FF4D9: Perl_pp_multiply (pp.c:1274) ==6140== by 0x83506FA: Perl_runops_standard (run.c:41) ==6140== by 0x8116C46: S_run_body (perl.c:2456) ==6140== by 0x8116C46: perl_run (perl.c:2379) ==6140== by 0x8068761: main (perlmain.c:116) ==6140== Address 0x42bffd4 is 4 bytes before a block of size 512 alloc'd ==6140== at 0x402A0DE: malloc (vg_replace_malloc.c:296) ==6140== by 0x82B3593: Perl_safesysmalloc (util.c:153) ==6140== by 0x83435E7: Perl_av_extend_guts (av.c:182) ==6140== by 0x834375F: Perl_av_extend (av.c:80) ==6140== by 0x845C97D: Perl_new_stackinfo (scope.c:56) ==6140== by 0x80FF36D: Perl_init_stacks (perl.c:4063) ==6140== by 0x80FF84D: perl_construct (perl.c:249) ==6140== by 0x80684B8: main (perlmain.c:110) ==6140== ==6140== Invalid read of size 4 ==6140== at 0x83FF152: Perl_pp_multiply (pp.c:1275) ==6140== by 0x83506FA: Perl_runops_standard (run.c:41) ==6140== by 0x8116C46: S_run_body (perl.c:2456) ==6140== by 0x8116C46: perl_run (perl.c:2379) ==6140== by 0x8068761: main (perlmain.c:116) ==6140== Address 0x8 is not stack'd\, malloc'd or (recently) free'd ==6140== ==6140== ==6140== Process terminating with default action of signal 11 (SIGSEGV) ==6140== Access not within mapped region at address 0x8 ==6140== at 0x83FF152: Perl_pp_multiply (pp.c:1275) ==6140== by 0x83506FA: Perl_runops_standard (run.c:41) ==6140== by 0x8116C46: S_run_body (perl.c:2456) ==6140== by 0x8116C46: perl_run (perl.c:2379) ==6140== by 0x8068761: main (perlmain.c:116) ==6140== If you believe this happened as a result of a stack ==6140== overflow in your program's main thread (unlikely but ==6140== possible)\, you can try to increase the size of the ==6140== main thread stack using the --main-stacksize= flag. ==6140== The main thread stack size used in this run was 8388608. ==6140== ==6140== HEAP SUMMARY: ==6140== in use at exit: 87\,294 bytes in 520 blocks ==6140== total heap usage: 618 allocs\, 98 frees\, 104\,737 bytes allocated ==6140== ==6140== LEAK SUMMARY: ==6140== definitely lost: 80 bytes in 1 blocks ==6140== indirectly lost: 1\,821 bytes in 18 blocks ==6140== possibly lost: 0 bytes in 0 blocks ==6140== still reachable: 85\,393 bytes in 501 blocks ==6140== suppressed: 0 bytes in 0 blocks ==6140== Rerun with --leak-check=full to see details of leaked memory ==6140== ==6140== For counts of detected and suppressed errors\, rerun with: -v ==6140== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) Segmentation fault

p5pRT commented 9 years ago

From @dcollinsn

For fun\, some other examples of failing testcases after afl-tmin:

&{0==&{0==0}}.0 &{0==0}/&{0==0}&0 &{0*&{0==0}==0}/0 &{0==0}*&{0==0}

p5pRT commented 9 years ago

From @cpansprout

On Sat Sep 12 05:36:20 2015\, dcollinsn@gmail.com wrote:

&{0==0}*&{0==0}

Several other variations of this failed\, but this is the simplest example. All have either two calls to "&{0==0}" or nested calls of that type "&{0==&{0==0}}". This looks vaguely similar to the bugs arising from the stack not being refcounted (c.f. 125840\, 123710\, 123804\, 123997\, but the failure point is different.

No\, it’s more like stack corruption (perl getting confused about where it’s stack pointer should be):

$ ./miniperl -Ilib -le 'print 1\, 2\, 3\,&{0==0}*&{0==0}\, 4\, 5\, 6' 16456

--

Father Chrysostomos

p5pRT commented 9 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 9 years ago

From @cpansprout

On Sun Sep 20 15:22:29 2015\, sprout wrote:

On Sat Sep 12 05:36:20 2015\, dcollinsn@gmail.com wrote:

&{0==0}*&{0==0}

Several other variations of this failed\, but this is the simplest example. All have either two calls to "&{0==0}" or nested calls of that type "&{0==&{0==0}}". This looks vaguely similar to the bugs arising from the stack not being refcounted (c.f. 125840\, 123710\, 123804\, 123997\, but the failure point is different.

No\, it’s more like stack corruption (perl getting confused about where it’s stack pointer should be):

$ ./miniperl -Ilib -le 'print 1\, 2\, 3\,&{0==0}*&{0==0}\, 4\, 5\, 6' 16456

Specifically\, it is related to the fact that PL_sv_yes->() silently does nothing\, but its doing nothing is badly implemented.

This whole PL_sv_yes call thing is wart resulting from implementation details. I wonder if we can just remove it from perl space. Surely nobody is relying on it! (Famous last words.)

--

Father Chrysostomos

p5pRT commented 9 years ago

From zefram@fysh.org

Father Chrysostomos via RT wrote:

This whole PL_sv_yes call thing is wart resulting from implementation details. I wonder if we can just remove it from perl space.

We certainly should\, because its exposure per se produces visible bugs:

$ perl -lwe '*1 = sub { print @_; }; &1(2); &{!!1}(3)' 2

Even if its do-nothing behaviour were implemented correctly\, it's wrong for it to behave differently from an ordinary 1 as a subroutine ref.

-zefram

p5pRT commented 8 years ago

From @tonycoz

On Sat Sep 12 05:36:20 2015\, dcollinsn@gmail.com wrote:

After reducing testcases using `afl-tmin` and filtering out testcases that are merely iterations of "#!perl -u"\, I have located the following testcase that triggers a segmentation fault in the perl interpreter. The testcase is the 15-byte file:

&{0==0}*&{0==0}

The attached fixes the handling of calls to &PL_sv_yes.

I'm not sure how to approach eliminating the wart.

Tony

p5pRT commented 8 years ago

From @tonycoz

0001-perl-126042-handle-scalar-context-for-a-missing-impo.patch

```diff From d3e254d742fc2792ecc5e19a1fa4f1a3fa79f57b Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Wed, 4 Nov 2015 11:03:36 +1100 Subject: [perl #126042] handle scalar context for a missing import/unimport call --- pp_hot.c | 2 ++ t/op/method.t | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pp_hot.c b/pp_hot.c index 87e306c..366828a 100644 --- a/pp_hot.c +++ b/pp_hot.c @@ -3275,6 +3275,8 @@ PP(pp_entersub) SP = PL_stack_base + POPMARK; else (void)POPMARK; + if (GIMME_V == G_SCALAR) + PUSHs(&PL_sv_undef); RETURN; } SvGETMAGIC(sv); diff --git a/t/op/method.t b/t/op/method.t index 1171f4a..0d7f254 100644 --- a/t/op/method.t +++ b/t/op/method.t @@ -13,7 +13,7 @@ BEGIN { use strict; no warnings 'once'; -plan(tests => 147); +plan(tests => 148); @A::ISA = 'B'; @B::ISA = 'C'; @@ -657,6 +657,23 @@ SKIP: { like ($@, qr/Modification of a read-only value attempted/, 'RT #123619'); } +{ + # RT #126042 &{1==1} * &{1==1} would crash + + # pp_entersub and pp_method_named cooperate to prevent calls to an + # undefined import() or unimport() method from croaking. + # If pp_method_named can't find the method it pushes &PL_sv_yes, and + # pp_entersub checks for that specific SV to avoid croaking. + # Ideally they wouldn't use that hack but... + # Unfortunately pp_entersub's handling of that case is broken in scalar context. + + # Rather than using the test case from the ticket, since &{1==1} + # isn't documented (and may not be supported in future perls) test + # calls to undefined import method, which also crashes. + fresh_perl_is('Unknown->import() * Unknown->unimport(); print "ok\n"', "ok\n", {}, + "check unknown import() methods don't corrupt the stack"); +} + __END__ #FF9900 #F78C08 -- 2.1.4 ```

p5pRT commented 8 years ago

From @bulk88

On Tue Nov 03 16:17:43 2015\, tonyc wrote:

The attached fixes the handling of calls to &PL_sv_yes.

I'm not sure how to approach eliminating the wart.

Tony

That fix looks like it doesn't fix the problem. It needs to be fixed in the optree at compile tile\, that multiply operator takes 1 argument\, not empty list. There might be other ways to get rh and lh operands of multiply to be empty list and cause the same crash again without calling empty sub references (PL_sv_yes in this case).

-- bulk88 ~ bulk88 at hotmail.com

p5pRT commented 8 years ago

From zefram@fysh.org

bulk88 via RT wrote:

There might be other ways to get rh and lh operands of multiply to be empty list and cause the same crash again

Any such way would be another bug. The operands of the multiply op are put into scalar context\, and it is intended that every optree scalar-context optree will yield exactly one value on the stack. The magic subroutine behind &PL_sv_yes was erroneous in not obeying scalar context.

-zefram

p5pRT commented 8 years ago

From @tonycoz

On Tue Nov 03 16:17:43 2015\, tonyc wrote:

On Sat Sep 12 05:36:20 2015\, dcollinsn@gmail.com wrote:

After reducing testcases using `afl-tmin` and filtering out testcases that are merely iterations of "#!perl -u"\, I have located the following testcase that triggers a segmentation fault in the perl interpreter. The testcase is the 15-byte file:

&{0==0}*&{0==0}

The attached fixes the handling of calls to &PL_sv_yes.

Applied as 0cd52e23ae641a766983833dc8b37b743474be9e.

I'm not sure how to approach eliminating the wart.

Leaving the ticket open in case someone figures that out.

Tony

p5pRT commented 8 years ago

From @iabyn

On Wed\, Jan 06\, 2016 at 09:28:17PM -0800\, Tony Cook via RT wrote:

I'm not sure how to approach eliminating the wart.

Leaving the ticket open in case someone figures that out.

Maybe make the method-locating code return a stub XS CV rather than PL_sv_yes when it fails to find "import"?\, This XS sub would do whatever pp_entersub used to do when it encountered PL_sv_yes.

-- "Procrastination grows to fill the available time" -- Mitchell's corollary to Parkinson's Law

p5pRT commented 6 years ago

From zefram@fysh.org

Dave Mitchell wrote:

Maybe make the method-locating code return a stub XS CV rather than PL_sv_yes when it fails to find "import"?

Done in commit 0740a29d60ebd4ff72090340b0140ec2210e90c7. That resolves this ticket.

-zefram

p5pRT commented 6 years ago

@iabyn - Status changed from 'open' to 'pending release'

p5pRT commented 6 years ago

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release yesterday of Perl 5.28.0\, this and 185 other issues have been resolved.

Perl 5.28.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.28.0

If you find that the problem persists\, feel free to reopen this ticket.

p5pRT commented 6 years ago

@khwilliamson - Status changed from 'pending release' to 'resolved'

Perl / perl5