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

Owner: Nobody
Requestors: hv <hv [at] crypt.org>
Cc:
AdminCc:

Operating System: Linux
PatchStatus: (no value)
Severity: medium
Type: core
Perl Version: 5.21.9
Fixed In: 5.22.0



Date: Fri, 06 Feb 2015 16:59:10 +0000
From: hv [...] crypt.org
To: perlbug [...] perl.org
Subject: op_private assert
CC: hv [...] crypt.org
This is a bug report for perl from hv@crypt.org, generated with the help of perlbug 1.40 running under perl 5.21.9. ----------------------------------------------------------------- [Please describe your issue here] The check to "Assert valid op_private bits in op_free()" added by Dave in d0c8136d16 can be triggered by the temporary poking of numargs into op_private by ck_fun(), if a syntax error causes unwinding before the field has been reset (presumably by a later ck_* function). My shortest test case is: % ./miniperl -e 'map+map' Not enough arguments for map at -e line 1, at EOF Execution of -e aborted due to compilation errors. miniperl: op.c:721: Perl_op_free: Assertion `!(o->op_private & ~PL_op_private_valid[type])' failed. Aborted (core dumped) % In eb8433b7af, Nick added a FIXME comment asking whether the storing of numargs should move after the "too many args" check, but that wouldn't help us in this case - first because it's the later "too few args" check we fail on, and second because the mapstart op we assert on isn't the one for which we've generated the "Not enough arguments" error. I'm not sure how to search for what uses the numargs; ideally we'd remove the need to piggyback on op_private in this way. (I've been trying out American Fuzzy Lop: this represents a high proportion of the failure cases I've seen so far. It also finds cases triggering the same assert for different reasons on an RV2CV, but I haven't looked at those yet.) [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=medium --- Site configuration information for perl 5.21.9: Configured by hv at Fri Feb 6 16:50:59 GMT 2015. Summary of my perl5 (revision 5 version 21 subversion 9) configuration: Commit id: eb82332cb71f48a5a63aa48dda0f6f55ee333ecb Platform: osname=linux, osvers=3.13.0-37-generic, archname=x86_64-linux uname='linux shad2 3.13.0-37-generic #64-ubuntu smp mon sep 22 21:28:38 utc 2014 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Dcc=gcc -Dprefix=/opt/blead-d -Doptimize=-g -O6 -DDEBUGGING -Dusedevel -Uversiononly' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-g -O6', cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.8.2', 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='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='gcc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.8/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.19.so, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.19' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -g -O6 -L/usr/local/lib -fstack-protector' --- @INC for perl 5.21.9: /opt/blead-d/lib/perl5/site_perl/5.21.9/x86_64-linux /opt/blead-d/lib/perl5/site_perl/5.21.9 /opt/blead-d/lib/perl5/5.21.9/x86_64-linux /opt/blead-d/lib/perl5/5.21.9 . --- Environment for perl 5.21.9: HOME=/home/hv LANG=C LANGUAGE=en_GB:en LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/hv/bin:/home/hv/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games PERL_BADLANG (unset) SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 341b
On Fri Feb 06 08:59:56 2015, hv wrote: Show quoted text
> I'm not sure how to search for what uses the numargs;
Anything using MAXARG uses that. Most ops that take one or two arguments already have the lower bits set to the number of operands by newUNOP or newBINOP. But index, for instance, depends on ck_fun to set those bits. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.5k
On Fri Feb 06 08:59:56 2015, hv wrote: Show quoted text
> This is a bug report for perl from hv@crypt.org, > generated with the help of perlbug 1.40 running under perl 5.21.9. > > > ----------------------------------------------------------------- > [Please describe your issue here] > > The check to "Assert valid op_private bits in op_free()" added by Dave > in d0c8136d16 can be triggered by the temporary poking of numargs into > op_private by ck_fun(), if a syntax error causes unwinding before the > field has been reset (presumably by a later ck_* function). > > My shortest test case is: > % ./miniperl -e 'map+map' > Not enough arguments for map at -e line 1, at EOF > Execution of -e aborted due to compilation errors. > miniperl: op.c:721: Perl_op_free: Assertion `!(o->op_private & > ~PL_op_private_valid[type])' failed. > Aborted (core dumped) > % > > In eb8433b7af, Nick added a FIXME comment asking whether the storing > of > numargs should move after the "too many args" check, but that wouldn't > help us in this case - first because it's the later "too few args" > check > we fail on, and second because the mapstart op we assert on isn't the > one > for which we've generated the "Not enough arguments" error.
I think it’s the assertion that’s wrong. Usually ck_fun sets the number of args, but ck_grep clears it. If an error occurs, it does not clear it (and there is no way to guarantee that it will, as any yyerror may croak), so the bit is left set. All we need to do is change the bit assignments in regen/op_private, which I plan to do soon. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.8k
On Fri Feb 06 10:02:51 2015, sprout wrote: Show quoted text
> On Fri Feb 06 08:59:56 2015, hv wrote:
> > This is a bug report for perl from hv@crypt.org, > > generated with the help of perlbug 1.40 running under perl 5.21.9. > > > > > > ----------------------------------------------------------------- > > [Please describe your issue here] > > > > The check to "Assert valid op_private bits in op_free()" added by > > Dave > > in d0c8136d16 can be triggered by the temporary poking of numargs > > into > > op_private by ck_fun(), if a syntax error causes unwinding before the > > field has been reset (presumably by a later ck_* function). > > > > My shortest test case is: > > % ./miniperl -e 'map+map' > > Not enough arguments for map at -e line 1, at EOF > > Execution of -e aborted due to compilation errors. > > miniperl: op.c:721: Perl_op_free: Assertion `!(o->op_private & > > ~PL_op_private_valid[type])' failed. > > Aborted (core dumped) > > % > > > > In eb8433b7af, Nick added a FIXME comment asking whether the storing > > of > > numargs should move after the "too many args" check, but that > > wouldn't > > help us in this case - first because it's the later "too few args" > > check > > we fail on, and second because the mapstart op we assert on isn't the > > one > > for which we've generated the "Not enough arguments" error.
> > I think it’s the assertion that’s wrong. Usually ck_fun sets the > number of args, but ck_grep clears it. If an error occurs, it does > not clear it (and there is no way to guarantee that it will, as any > yyerror may croak), so the bit is left set. All we need to do is > change the bit assignments in regen/op_private, which I plan to do > soon.
I’ve fixed this in fb0c7c3c47fb1. I was not able to come up with any cases that failed with other lower bits set. Do the other cases you mentioned still fail? -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 475b
On Fri Feb 06 10:17:02 2015, sprout wrote: Show quoted text
> I’ve fixed this in fb0c7c3c47fb1. I was not able to come up with any > cases that failed with other lower bits set. Do the other cases you > mentioned still fail?
Heh, I was about to say no, but more careful checking shows that only 12 of the 20 RV2CV cases now pass. I'm attaching to the ticket a tar file of the 8 that still fail - they all contain nul bytes and similar oddities. The 22 mapstart cases all pass now. Hugo
Subject: rv2cv-assert.tar
Download rv2cv-assert.tar
application/x-tar 10k

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 727b
On Fri Feb 06 10:43:18 2015, hv wrote: Show quoted text
> On Fri Feb 06 10:17:02 2015, sprout wrote:
> > I’ve fixed this in fb0c7c3c47fb1. I was not able to come up with any > > cases that failed with other lower bits set. Do the other cases you > > mentioned still fail?
> > Heh, I was about to say no, but more careful checking shows that only > 12 of the 20 RV2CV cases now pass. > > I'm attaching to the ticket a tar file of the 8 that still fail - they > all contain nul bytes and similar oddities. The 22 mapstart cases all > pass now.
The first of those is definitely memory corruption. It crashes intermittently. If I dump the op before the assertion I get a different set of private flags each time. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.8k
On Fri Feb 06 12:56:03 2015, sprout wrote: Show quoted text
> The first of those is definitely memory corruption.
Ah, I have caught something in the act: (gdb) cont Hardware watchpoint 3: ((UNOP*)0xa81898)->op_private Old value = 0 '\000' New value = 25 '\031' Perl_newUNOP (type=17, flags=11016408, first=0xa81918) at op.c:4811 4811 unop = (UNOP*) CHECKOP(type, unop); (gdb) # ^^^^^ so it triggered on 'unop->op_private = (U8)(1 | (flags >> 8));' (gdb) p /x flags $22 = 0xa818d8 (gdb) # ^^^^^ that's almost certainly an OP address, not flags (gdb) where #0 Perl_newUNOP (type=17, flags=11016408, first=0xa81918) at op.c:4811 #1 0x0000000000446b80 in Perl_newCVREF (flags=11016408, o=0xa81918) at op.c:9377 #2 0x00000000004d309a in Perl_yyparse (gramtype=258) at perly.y:1105 #3 0x000000000045f79a in S_parse_body (env=0x0, xsinit=0x41ef3b <xs_init>) at perl.c:2273 #4 0x000000000045dde7 in perl_parse (my_perl=0xa5d010, xsinit=0x41ef3b <xs_init>, argc=2, argv=0x7fffffffe658, env=0x0) at perl.c:1607 #5 0x000000000041ee9d in main (argc=2, argv=0x7fffffffe658, env=0x7fffffffe670) at perlmain.c:114 So we're managing to call newCVREF via this bit of perly: amper : '&' indirob { $$ = newCVREF($1,$2); } .. but $1 is an op address instead of a sane set of flags. (Note that there also seems to be some reuse of op bodies going on: the same watchpoint was getting triggered by the Zero() at op.c:250.) For ease of reference, here's a hex dump of the program: --- id:000001,sig:11,src:001079,op:havoc,rep:8 000000 22 61 62 7d 22 61 78 3b 26 00 7a 8a 6f 7d 82 78 "ab}"ax;&.z.o}.x 000010 3b 0a ;. It'll take me a while to dig further, I haven't had to pretend not to be scared of perly.y for a good 10 years. But hey, I think that means the new assert has found something really useful. :)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Fri Feb 06 12:56:03 2015, sprout wrote: Show quoted text
> The first of those is definitely memory corruption. It crashes > intermittently. If I dump the op before the assertion I get a > different set of private flags each time.
No, not memory corruption. Perl’s parser tolerates stray nulls, effectively treating them as whitespace, or so I thought.... $ perl -e 'print qq|use strict; "a";"a";"a";&\0e|' | ./perl -Ilib Undefined subroutine &main::e called at - line 1. $ perl -e 'print qq|use strict; "a";"a";"a";"a";&\0e|' | ./perl -Ilib Bareword "e" not allowed while "strict subs" in use at - line 1. Execution of - aborted due to compilation errors. $ perl -e 'print qq|&\0eq|' | ./perl -Ilib syntax error at - line 1, near "&eq" Execution of - aborted due to compilation errors. There the lexer produces '&' EQOP instead of '&' WORD. A memory address (the last value in pl_yylval) is being stuffed into the flags field, which explains the intermittency. Usually it is harmless (it happens for &{...}, which seems to be impervious to it), but &\0foo results in unexpected code paths being followed. The nulls should probably be skipped when the identifier after & is scanned. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.1k
On Fri Feb 06 17:21:30 2015, hv wrote: Show quoted text
> On Fri Feb 06 12:56:03 2015, sprout wrote:
> > The first of those is definitely memory corruption.
> > Ah, I have caught something in the act: > > (gdb) cont > Hardware watchpoint 3: ((UNOP*)0xa81898)->op_private > > Old value = 0 '\000' > New value = 25 '\031' > Perl_newUNOP (type=17, flags=11016408, first=0xa81918) at op.c:4811 > 4811 unop = (UNOP*) CHECKOP(type, unop); > (gdb) # ^^^^^ so it triggered on 'unop->op_private = (U8)(1 | (flags
> >> 8));'
> (gdb) p /x flags > $22 = 0xa818d8 > (gdb) # ^^^^^ that's almost certainly an OP address, not flags > (gdb) where > #0 Perl_newUNOP (type=17, flags=11016408, first=0xa81918) at > op.c:4811 > #1 0x0000000000446b80 in Perl_newCVREF (flags=11016408, o=0xa81918) > at op.c:9377 > #2 0x00000000004d309a in Perl_yyparse (gramtype=258) at perly.y:1105 > #3 0x000000000045f79a in S_parse_body (env=0x0, xsinit=0x41ef3b > <xs_init>) > at perl.c:2273 > #4 0x000000000045dde7 in perl_parse (my_perl=0xa5d010, > xsinit=0x41ef3b <xs_init>, argc=2, argv=0x7fffffffe658, env=0x0) > at perl.c:1607 > #5 0x000000000041ee9d in main (argc=2, argv=0x7fffffffe658, > env=0x7fffffffe670) at perlmain.c:114 > > So we're managing to call newCVREF via this bit of perly: > amper : '&' indirob > { $$ = newCVREF($1,$2); } > .. but $1 is an op address instead of a sane set of flags. (Note that > there also seems to be some reuse of op bodies going on: the same > watchpoint was getting triggered by the Zero() at op.c:250.) > > For ease of reference, here's a hex dump of the program: > --- id:000001,sig:11,src:001079,op:havoc,rep:8 > 000000 22 61 62 7d 22 61 78 3b 26 00 7a 8a 6f 7d 82 78 > "ab}"ax;&.z.o}.x > 000010 3b 0a > ;. > > It'll take me a while to dig further, I haven't had to pretend not to > be scared of perly.y for a good 10 years. But hey, I think that means > the new assert has found something really useful. :)
It’s the PREREF('&') in yylex that is not setting pl_yylval before returning. But, as I pointed out in my other message, &\0foo should ideally not even reach that code path. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.4k
On Fri Feb 06 18:13:31 2015, sprout wrote: Show quoted text
> On Fri Feb 06 17:21:30 2015, hv wrote:
> > On Fri Feb 06 12:56:03 2015, sprout wrote:
> > > The first of those is definitely memory corruption.
> > > > Ah, I have caught something in the act: > > > > (gdb) cont > > Hardware watchpoint 3: ((UNOP*)0xa81898)->op_private > > > > Old value = 0 '\000' > > New value = 25 '\031' > > Perl_newUNOP (type=17, flags=11016408, first=0xa81918) at op.c:4811 > > 4811 unop = (UNOP*) CHECKOP(type, unop); > > (gdb) # ^^^^^ so it triggered on 'unop->op_private = (U8)(1 | (flags
> > >> 8));'
> > (gdb) p /x flags > > $22 = 0xa818d8 > > (gdb) # ^^^^^ that's almost certainly an OP address, not flags > > (gdb) where > > #0 Perl_newUNOP (type=17, flags=11016408, first=0xa81918) at > > op.c:4811 > > #1 0x0000000000446b80 in Perl_newCVREF (flags=11016408, o=0xa81918) > > at op.c:9377 > > #2 0x00000000004d309a in Perl_yyparse (gramtype=258) at perly.y:1105 > > #3 0x000000000045f79a in S_parse_body (env=0x0, xsinit=0x41ef3b > > <xs_init>) > > at perl.c:2273 > > #4 0x000000000045dde7 in perl_parse (my_perl=0xa5d010, > > xsinit=0x41ef3b <xs_init>, argc=2, argv=0x7fffffffe658, env=0x0) > > at perl.c:1607 > > #5 0x000000000041ee9d in main (argc=2, argv=0x7fffffffe658, > > env=0x7fffffffe670) at perlmain.c:114 > > > > So we're managing to call newCVREF via this bit of perly: > > amper : '&' indirob > > { $$ = newCVREF($1,$2); } > > .. but $1 is an op address instead of a sane set of flags. (Note that > > there also seems to be some reuse of op bodies going on: the same > > watchpoint was getting triggered by the Zero() at op.c:250.) > > > > For ease of reference, here's a hex dump of the program: > > --- id:000001,sig:11,src:001079,op:havoc,rep:8 > > 000000 22 61 62 7d 22 61 78 3b 26 00 7a 8a 6f 7d 82 78 > > "ab}"ax;&.z.o}.x > > 000010 3b 0a > > ;. > > > > It'll take me a while to dig further, I haven't had to pretend not to > > be scared of perly.y for a good 10 years. But hey, I think that means > > the new assert has found something really useful. :)
> > It’s the PREREF('&') in yylex that is not setting pl_yylval before > returning. But, as I pointed out in my other message, &\0foo should > ideally not even reach that code path.
I have fixed this in 3c47da3c2eb. I still want to investigate whether stuffing an address into the flags still causes problems (mainly because I want to add tests for it if I fix it). -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 587b
On Sat Feb 07 10:11:05 2015, sprout wrote: Show quoted text
> On Fri Feb 06 18:13:31 2015, sprout wrote:
> > It’s the PREREF('&') in yylex that is not setting pl_yylval before > > returning. But, as I pointed out in my other message, &\0foo should > > ideally not even reach that code path.
> > I have fixed this in 3c47da3c2eb. I still want to investigate whether > stuffing an address into the flags still causes problems (mainly > because I want to add tests for it if I fix it).
I have managed to trigger an assertion failure with &{+foo}, which I fixed in eea89386b. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Sat Feb 07 10:11:05 2015, sprout wrote: Show quoted text
> I have fixed this in 3c47da3c2eb. I still want to investigate whether > stuffing an address into the flags still causes problems (mainly > because I want to add tests for it if I fix it).
I've restarted AFL with latest blead and a couple of tweaks, and it's doing much better; but after 9 hours it came up with this: % hd crashes/id* --- id:000000,sig:11,src:001996,op:havoc,rep:8 000000 61 24 24 24 3f 26 24 24 24 24 24 10 24 24 3b 19 a$$$?&$$$$$.$$;. 000010 24 $ % Its afl-tmin tool minimized that to this: % hd ~/crash --- /home/hv/crash 000000 24 30 3f 26 24 30 10 $0?&$0. % /opt/blead-d0/bin/perl ~/crash Unrecognized character \x10; marked by <-- HERE after $0?&$0<-- HERE near column 7 at /home/hv/crash line 1. % /opt/blead-d0/bin/perl ~/crash Unrecognized character \x10; marked by <-- HERE after $0?&$0<-- HERE near column 7 at /home/hv/crash line 1. perl: op.c:721: Perl_op_free: Assertion `!(o->op_private & ~PL_op_private_valid[type])' failed. Aborted (core dumped) % .. which looks like it's probably the same issue. Hugo
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 195b
On Sat Feb 07 20:39:09 2015, hv wrote: Show quoted text
> .. which looks like it's probably the same issue.
.. and which looks to be fixed by eea89386b3. (Sorry, I hadn't seen the update when I sent that.) Hugo
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