perl5
perl5 copied to clipboard
Perl
Taint-mode assert fail in Perl_magic_clearisa without other symptoms
From @dcollinsn
Greetings Porters,
I have compiled bleadperl with the afl-gcc compiler using:
./Configure -Dusedevel -Dprefix='/usr/local/perl-afl' -Dcc='ccache afl-gcc' -Uuselongdouble -Duse64bitall -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -Dusequadmath -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 performing additional minimization by hand, I have located the following testcase that triggers an assert fail in debug buids of the perl interpreter. The testcase is the file below. On normal builds, this runs normally (albeit with an expected warning). On debug builds, this returns an assert fail.
dcollins@nightshade64:~/perl$ ./perl -Ilib -tW -e '{@{*a::ISA}=undef*a::ISA;@a::ISA=0}' Use of uninitialized value in list assignment at -e line 1. dcollins@nightshade64:~/perl$ cd ../perldebug/
dcollins@nightshade64:~/perldebug$ ./perl -Ilib -tW -e '{@{*a::ISA}=undef*a::ISA;@a::ISA=0}' Use of uninitialized value in list assignment at -e line 1. perl: mg.c:1726: Perl_magic_clearisa: Assertion `((((_gvstash)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((_gvstash)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((_gvstash)->sv_flags & 0xff)) == SVt_PVLV))' failed. Aborted
Debugging tool output is below. A git bisect was performed and reported the following.
5e267fb87fe17363909b2ed49f916dccf9939c3b is the first bad commit commit 5e267fb87fe17363909b2ed49f916dccf9939c3b Author: David Mitchell <davem@iabyn.com> Date: Wed Oct 21 13:10:44 2015 +0100
Always copy return values when exiting scope
v5.14.0-642-g3ed94dc fixed certain instances where returning from a sub incorrectly returned the actual value rather than a copy, e.g.
sub f { delete $foo{bar} }
This was because if the value(s) being returned had SvTEMP set, copying was skipped. That commit added an extra condition to the skip test, SvREFCNT(sv) == 1.
However, this applies equally well to other scope exits, for example
do { ...; delete $foo{bar} }
So this commits adds the RC==1 test to S_leave_common() too, which handles all the non-sub scope exits. As well as adding a test to do.t, it adds an additional test to sub.t, since the original tests, although they *detected* a non-copied return, didn't actually demonstrate a case where it was actually harmful.
Note that S_leave_common() also sometimes skips on PADTMPs as well as TEMPs, so this commit as a side-effect also makes it copy PADTMPs unless their RC ==1. But since their RC should in fact almost always be 1 anyway, in practice it makes no difference.
:100644 100644 82189bb5c188e77800176fe8cfcb80b15bcb2226 d1229af7609c41d1caa7944f8d90eb6a2b12859c M pp_ctl.c :040000 040000 95d71e14d5e47e4db9a5149492215aa45b9ad6c8 f69fbca5ed4bd5f0595f03be7deda11cca858d43 M t bisect run success
**GDB**
dcollins@nightshade64:~/perldebug$ gdb --args ./perl -Ilib -tW -e '{@{*a::ISA}=undef*a::ISA;@a::ISA=0}' 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 "x86_64-unknown-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 ./perl...done. (gdb) run Starting program: /home/dcollins/perldebug/perl -Ilib -tW -e \{@\{\*a::ISA\}=undef\*a::ISA\;@a::ISA=0\} [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Use of uninitialized value in list assignment at -e line 1. perl: mg.c:1726: Perl_magic_clearisa: Assertion `((((_gvstash)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((_gvstash)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((_gvstash)->sv_flags & 0xff)) == SVt_PVLV))' failed.
Program received signal SIGABRT, Aborted. 0x00007ffff6cf9478 in raise () from /lib/x86_64-linux-gnu/libc.so.6 (gdb) bt #0 0x00007ffff6cf9478 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007ffff6cfa8fa in abort () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x00007ffff6cf23a7 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #3 0x00007ffff6cf2452 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6 #4 0x000000000057548f in Perl_magic_clearisa (sv=0x0, mg=0xabc3e0) at mg.c:1724 #5 0x0000000000575104 in Perl_magic_setisa (sv=0xab20b0, mg=0xabc3e0) at mg.c:1691 #6 0x000000000056dfa1 in Perl_mg_set (sv=0xab20b0) at mg.c:277 #7 0x00000000005aeacc in Perl_pp_aassign () at pp_hot.c:1422 #8 0x000000000055a245 in Perl_runops_debug () at dump.c:2239 #9 0x00000000004623d3 in S_run_body (oldscope=1) at perl.c:2517 #10 0x00000000004619fe in perl_run (my_perl=0xa9c010) at perl.c:2440 #11 0x000000000041eae0 in main (argc=5, argv=0x7fffffffe5f8, env=0x7fffffffe628) at perlmain.c:116 (gdb) f 4 #4 0x000000000057548f in Perl_magic_clearisa (sv=0x0, mg=0xabc3e0) at mg.c:1724 1724 stash = GvSTASH( (gdb) l 1719 } 1720 1721 return 0; 1722 } 1723 1724 stash = GvSTASH( 1725 (const GV *)mg->mg_obj 1726 ); 1727 1728 /* The stash may have been detached from the symbol table, so check its (gdb) info locals _gvstash = 0xab2068 stash = 0xab20b0 __PRETTY_FUNCTION__ = "Perl_magic_clearisa"
**VALGRIND**
No reported memory management errors.
**PERL -V**
dcollins@nightshade64:~/perldebug$ ./perl -Ilib -V Summary of my perl5 (revision 5 version 25 subversion 2) configuration: Commit id: c29dfc6a6c45f86648c51f961304254cc3c449b9 Platform: osname=linux, osvers=4.5.0-2-amd64, archname=x86_64-linux-ld uname='linux nightshade64 4.5.0-2-amd64 #1 smp debian 4.5.3-2 (2016-05-08) x86_64 gnulinux ' config_args='-Dusedevel -Dprefix=/usr/local/perl-afl -Dcc=ccache gcc-6.1 -Duselongdouble -Duse64bitall -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -DDEBUGGING -DPERL_POISON -des' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef use64bitint=define, use64bitall=define, uselongdouble=define usemymalloc=n, bincompat5005=undef Compiler: cc='ccache gcc-6.1', ccflags ='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-g', cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='', gccversion='6.1.0', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3 ivtype='long', ivsize=8, nvtype='long double', nvsize=16, Off_t='off_t', lseeksize=8 alignbytes=16, prototype=define Linker and Libraries: ld='ccache gcc-6.1', ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/local/lib/gcc/x86_64-pc-linux-gnu/6.1.0/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib 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: DEBUGGING HAS_TIMES PERLIO_LAYERS PERL_COPY_ON_WRITE PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP PERL_OP_PARENT PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_ALL 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 May 26 2016 17:57:37 @INC: lib /usr/local/perl-afl/lib/site_perl/5.25.2/x86_64-linux-ld /usr/local/perl-afl/lib/site_perl/5.25.2 /usr/local/perl-afl/lib/5.25.2/x86_64-linux-ld /usr/local/perl-afl/lib/5.25.2 /usr/local/perl-afl/lib/site_perl/5.25.1 /usr/local/perl-afl/lib/site_perl/5.24.0 /usr/local/perl-afl/lib/site_perl .
From @iabyn
On Thu, May 26, 2016 at 05:29:21PM -0700, Dan Collins wrote:
dcollins@nightshade64:~/perldebug$ ./perl -Ilib -tW -e '{@{*a::ISA}=undef*a::ISA;@a::ISA=0}' Use of uninitialized value in list assignment at -e line 1. perl: mg.c:1726: Perl_magic_clearisa: Assertion `((((_gvstash)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((_gvstash)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((_gvstash)->sv_flags & 0xff)) == SVt_PVLV))' failed. Aborted
It's equivalent to the following:
perl -t run against:
undef *a::ISA; @{*a::ISA}=(); @a::ISA=0
The problem is in the weak pointer from ISA magic to the GV, i.e. where the *a::ISA points to the GP which points to @a::ISA which has ISA magic attached, who's mg_obj field contains a weak pointer back to *a::ISA. It has to be weak, otherwise there would be a reference loop and leakage.
However, with this: @{*a::ISA}, the *a::ISA expression is executed within a block scope; when that scope is left, a copy of the scope's return value is returned (as is done with all rvalue scope exits). This copying creates an SvTEMP GV whose GP is shared with the original *a::ISA GV. When the gp_av slot of the temp GV is accessed, it is empty due to the earlier undef, so is autovivified. The autovivification sees that the GV's name is 'ISA' so attaches ISA magic, whose mg_obj field is set to point (weakly) to the temp GV copy of *a::ISA.
At the next statement boundary temps are freed, and @a::ISA's mg_obj field now points to a freed GV. Crashes etc follow.
I don't know how to fix this.
-- Dave's first rule of Opera: If something needs saying, say it: don't warble it.
From [email protected]
Dave Mitchell wrote:
I don't know how to fix this.
The GP has a pointer to the interned (`effective') GV from which any GV referencing it was cloned. Attaching ISA magic should follow that pointer.
-zefram
From @cpansprout
On Mon Jun 27 08:54:23 2016, davem wrote:
It's equivalent to the following:
perl -t run against:
undef *a::ISA; @{*a::ISA}=(); @a::ISA=0
Why does it only happen under -t and -T?
--
Father Chrysostomos
From @iabyn
On Mon, Jun 27, 2016 at 05:58:15PM -0700, Father Chrysostomos via RT wrote:
On Mon Jun 27 08:54:23 2016, davem wrote:
It's equivalent to the following:
perl -t run against:
undef *a::ISA; @{*a::ISA}=(); @a::ISA=0
Why does it only happen under -t and -T?
I just checked. In the absence of -t, the @{*a::ISA} is compiled with a scope rather than enter/leave. Changing it to something like @{my $x; *a::ISA} to force full scope, causes it to crash without the -t.
-- Any [programming] language that doesn't occasionally surprise the novice will pay for it by continually surprising the expert. -- Larry Wall
From @iabyn
On Mon, Jun 27, 2016 at 05:27:05PM +0100, Zefram wrote:
Dave Mitchell wrote:
I don't know how to fix this.
The GP has a pointer to the interned (`effective') GV from which any GV referencing it was cloned. Attaching ISA magic should follow that pointer.
That won't work in a case like:
*Bar::ISA = *Foo::ISA
When the ISA array is modified, both stashes need mro_isa_changed_in() called on them, but there's only a single GP involved.
At the moment, mg_obj normally points to the ISA AV's GV, but when there are multiple GVs, as in the above or *Bar::ISA = \@Foo::ISA, then mg_obj points to a non-reified AV whose elements are the parent GVs.
But the current scheme will (I imagine) fail in the case of something like
$Bar::{ISA} = $Foo::{ISA};
Perhaps instead of storing GV or GP pointer(s) in the ISA magic, we should store stash names (i.e. a PV or an AV of PV's). Then when @ISA is modified, the stashes are looked up from the stash names.
-- Little fly, thy summer's play my thoughtless hand has terminated with extreme prejudice. (with apologies to William Blake)
From @cpansprout
On Tue Jun 28 02:35:26 2016, davem wrote:
On Mon, Jun 27, 2016 at 05:27:05PM +0100, Zefram wrote:
Dave Mitchell wrote:
I don't know how to fix this.
The GP has a pointer to the interned (`effective') GV from which any GV referencing it was cloned. Attaching ISA magic should follow that pointer.
That won't work in a case like:
\*Bar​::ISA = \*Foo​::ISAWhen the ISA array is modified, both stashes need mro_isa_changed_in() called on them, but there's only a single GP involved.
At the moment, mg_obj normally points to the ISA AV's GV, but when there are multiple GVs, as in the above or *Bar::ISA = \@Foo::ISA, then mg_obj points to a non-reified AV whose elements are the parent GVs.
But the current scheme will (I imagine) fail in the case of something like
$Bar​::\{ISA\} = $Foo​::\{ISA\};
It’s practically impossible for perl to track things correctly when people do direct stash assignment. (That’s partly why perlmod says:
The results of creating new symbol table entries directly or modifying any entries that are not already typeglobs are undefined and subject to change between releases of perl.
)
So as long as that case does not crash, I think that’s fine.
Perhaps instead of storing GV or GP pointer(s) in the ISA magic, we should store stash names (i.e. a PV or an AV of PV's). Then when @ISA is modified, the stashes are looked up from the stash names.
As long as things still work when stashes get effectively renamed (Bar gets renamed in *Foo::=*Bar::, *Bar::=*Baz::). I went to great lengths to make sure that worked (in 5.12, iirc), and I would hate to see it break.
--
Father Chrysostomos