Stack overflow when local'izing readonly arrays

[p5pRT]

Migrated from rt.perl.org#134028 (status was 'open')

Searchable as RT134028$

[p5pRT]

From @dur-randir

Created by @dur-randir

While fuzzing perl v5.29.9-63-g2496d8f3f7 built with afl and run under libdislocator\, I found the following program

local@​-[0..7000]

to cause stack overflow. GDB stack trace is following

#1 0x00005555557d0110 in Perl_sv_vsetpvfn (sv=0x555555d70a78\, pat=0x555555aa220f "%s"\, patlen=2\, args=0x7fffff7ff480\, svargs=0x0\, sv_count=0\,   maybe_tainted=0x0) at sv.c​:10977 #2 0x0000555555710f4a in Perl_vmess (pat=0x555555aa220f "%s"\, args=0x7fffff7ff480) at util.c​:1484 #3 0x0000555555712025 in Perl_vcroak (pat=0x555555aa220f "%s"\, args=0x7fffff7ff480) at util.c​:1697 #4 0x000055555571231b in Perl_croak (pat=0x555555aa220f "%s") at util.c​:1744 #5 0x0000555555712337 in Perl_croak_no_modify () at util.c​:1762 #6 0x0000555555757d4e in Perl_av_delete (av=0x555555b74e50\, key=2194\, flags=4) at av.c​:894 #7 0x000055555581f9c7 in Perl_leave_scope (base=0) at scope.c​:1275 #8 0x000055555582de6d in Perl_dounwind (cxix=-1) at pp_ctl.c​:1550 #9 0x00005555555f9c55 in S_my_exit_jump () at perl.c​:5262 #10 0x00005555555f9ab8 in Perl_my_failure_exit () at perl.c​:5249 #11 0x000055555582f1e9 in Perl_die_unwind (msv=0x555555d70a60) at pp_ctl.c​:1797 #12 0x000055555571226a in Perl_vcroak (pat=0x555555aa220f "%s"\, args=0x7fffff7ffb50) at util.c​:1699 #13 0x000055555571231b in Perl_croak (pat=0x555555aa220f "%s") at util.c​:1744 #14 0x0000555555712337 in Perl_croak_no_modify () at util.c​:1762 #15 0x0000555555757d4e in Perl_av_delete (av=0x555555b74e50\, key=2195\, flags=4) at av.c​:894 ... #43261 0x000055555581f9c7 in Perl_leave_scope (base=0) at scope.c​:1275 1275 (void)av_delete(a1.any_av\, a0.any_iv\, G_DISCARD); #43262 0x0000555555832128 in Perl_pp_leave () at pp_ctl.c​:2136 2136 CX_LEAVE_SCOPE(cx);

Perl Info

``` Flags: category=core severity=low Site configuration information for perl 5.29.9: Configured by dur-randir at Wed Feb 27 14:51:01 MSK 2019. Summary of my perl5 (revision 5 version 29 subversion 9) configuration: Commit id: c1e47bad34ce1d9c84ed57c9b8978bcbd5a02e98 Platform: osname=darwin osvers=13.4.0 archname=darwin-thread-multi-2level uname='darwin isengard.local 13.4.0 darwin kernel version 13.4.0: mon jan 11 18:17:34 pst 2016; root:xnu-2422.115.15~1release_x86_64 x86_64 ' config_args='-de -Dusedevel -DDEBUGGING -Dusethreads' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='cc' ccflags ='-fno-common -DPERL_DARWIN -mmacosx-version-min=10.9 -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -DPERL_USE_SAFE_PUTENV' optimize='-O3 -g' cppflags='-fno-common -DPERL_DARWIN -mmacosx-version-min=10.9 -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='' gccversion='4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)' 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='cc' ldflags =' -mmacosx-version-min=10.9 -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/6.0/lib /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib /usr/lib libs=-lpthread -lgdbm -ldbm -ldl -lm -lutil -lc perllibs=-lpthread -ldl -lm -lutil -lc libc= so=dylib useshrplib=false libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=bundle d_dlsymun=undef ccdlflags=' ' cccdlflags=' ' lddlflags=' -mmacosx-version-min=10.9 -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector' @INC for perl 5.29.9: lib /usr/local/lib/perl5/site_perl/5.29.9/darwin-thread-multi-2level /usr/local/lib/perl5/site_perl/5.29.9 /usr/local/lib/perl5/5.29.9/darwin-thread-multi-2level /usr/local/lib/perl5/5.29.9 Environment for perl 5.29.9: DYLD_LIBRARY_PATH (unset) HOME=/Users/dur-randir LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-5.22.1/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/texbin PERLBREW_HOME=/Users/dur-randir/.perlbrew PERLBREW_MANPATH=/Users/dur-randir/perlbrew/perls/perl-5.22.1/man PERLBREW_PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-5.22.1/bin PERLBREW_PERL=perl-5.22.1 PERLBREW_ROOT=/Users/dur-randir/perlbrew PERLBREW_SHELLRC_VERSION=0.84 PERLBREW_VERSION=0.84 PERL_BADLANG (unset) SHELL=/usr/local/bin/zsh ```

[p5pRT]

From @tonycoz

On Sun\, 14 Apr 2019 03​:03​:11 -0700\, randir wrote​:

While fuzzing perl v5.29.9-63-g2496d8f3f7 built with afl and run under libdislocator\, I found the following program

local@​-[0..7000]

to cause stack overflow. GDB stack trace is following

#1 0x00005555557d0110 in Perl_sv_vsetpvfn (sv=0x555555d70a78\, pat=0x555555aa220f "%s"\, patlen=2\, args=0x7fffff7ff480\, svargs=0x0\, sv_count=0\, maybe_tainted=0x0) at sv.c​:10977 #2 0x0000555555710f4a in Perl_vmess (pat=0x555555aa220f "%s"\, args=0x7fffff7ff480) at util.c​:1484 #3 0x0000555555712025 in Perl_vcroak (pat=0x555555aa220f "%s"\, args=0x7fffff7ff480) at util.c​:1697 #4 0x000055555571231b in Perl_croak (pat=0x555555aa220f "%s") at util.c​:1744 #5 0x0000555555712337 in Perl_croak_no_modify () at util.c​:1762 #6 0x0000555555757d4e in Perl_av_delete (av=0x555555b74e50\, key=2194\, flags=4) at av.c​:894 #7 0x000055555581f9c7 in Perl_leave_scope (base=0) at scope.c​:1275 #8 0x000055555582de6d in Perl_dounwind (cxix=-1) at pp_ctl.c​:1550 #9 0x00005555555f9c55 in S_my_exit_jump () at perl.c​:5262 #10 0x00005555555f9ab8 in Perl_my_failure_exit () at perl.c​:5249 #11 0x000055555582f1e9 in Perl_die_unwind (msv=0x555555d70a60) at pp_ctl.c​:1797 #12 0x000055555571226a in Perl_vcroak (pat=0x555555aa220f "%s"\, args=0x7fffff7ffb50) at util.c​:1699 #13 0x000055555571231b in Perl_croak (pat=0x555555aa220f "%s") at util.c​:1744 #14 0x0000555555712337 in Perl_croak_no_modify () at util.c​:1762 #15 0x0000555555757d4e in Perl_av_delete (av=0x555555b74e50\, key=2195\, flags=4) at av.c​:894 ... #43261 0x000055555581f9c7 in Perl_leave_scope (base=0) at scope.c​:1275 1275 (void)av_delete(a1.any_av\, a0.any_iv\, G_DISCARD); #43262 0x0000555555832128 in Perl_pp_leave () at pp_ctl.c​:2136 2136 CX_LEAVE_SCOPE(cx);

Similarly for

  local@​-{0..7000}

This is caused by the av_delete() for SAVEt_ADELETE croaking due to @​- being readonly.

So it tries to "restore" element 7000 calling av_delete()\, which croaks\, and then tries to unwind the scope\, trying to delete element 6999\, which croaks and so on until we run out of stack.

We could fail earlier for the test case by throwing errors in aslice and hslice if we're localising a readonly array/slice\, but this won't help for the more general case since the array/hash might be set readonly after the local.

This would need to be done in many ops that localise for a more comprehensive fix\, though most of those won't localise in bulk like aslice/hslice. ( git grep LVAL_INTRO pp*.c )

Tony

[p5pRT]

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

[p5pRT]

From @tonycoz

On Mon\, 22 Apr 2019 18​:19​:56 -0700\, tonyc wrote​:

We could fail earlier for the test case by throwing errors in aslice and hslice if we're localising a readonly array/slice\, but this won't help for the more general case since the array/hash might be set readonly after the local.

The aslice case turned out to be simple.

The hslice case is not so simple\, since at least one test re-ties %- for testing\, which doesn't work on a read-only hash.

Tony

[p5pRT]

From @tonycoz

0001-perl-134028-disallow-localising-slices-of-a-RO-hash-.patch

```diff From f27364453c2e45a51516c6705bcc61277c6faa78 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Tue, 23 Apr 2019 20:15:42 +1000 Subject: (perl #134028) disallow localising slices of a RO hash/array This patch shouldn't be applied. Currently it causes re/subst.t to fail since it marks %-/%+ read-only which disallows the tie for the RT #23624 test. It also needs its own tests. --- ext/Tie-Hash-NamedCapture/NamedCapture.xs | 1 + pp.c | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/ext/Tie-Hash-NamedCapture/NamedCapture.xs b/ext/Tie-Hash-NamedCapture/NamedCapture.xs index 7eaae5614d..9d24321a55 100644 --- a/ext/Tie-Hash-NamedCapture/NamedCapture.xs +++ b/ext/Tie-Hash-NamedCapture/NamedCapture.xs @@ -32,6 +32,7 @@ _tie_it(SV *sv) sv_unmagic((SV *)hv, PERL_MAGIC_tied); sv_magic((SV *)hv, rv, PERL_MAGIC_tied, NULL, 0); + SvREADONLY_on((SV *)hv); SvREFCNT_dec(rv); /* As sv_magic increased it by one. */ SV * diff --git a/pp.c b/pp.c index babf34843e..d57a622936 100644 --- a/pp.c +++ b/pp.c @@ -4870,6 +4870,9 @@ PP(pp_aslice) MAGIC *mg; HV *stash; + if (SvREADONLY(av) && MARK < SP) + Perl_croak_no_modify(); + can_preserve = SvCANEXISTDELETE(av); } @@ -4903,6 +4906,9 @@ PP(pp_aslice) if (!svp || !*svp) DIE(aTHX_ PL_no_aelem, elem); if (localizing) { + if (SvREADONLY(*svp)) + Perl_croak_no_modify(); + if (preeminent) save_aelem(av, elem, svp); else @@ -5299,6 +5305,9 @@ PP(pp_hslice) MAGIC *mg; HV *stash; + if (SvREADONLY(hv) && MARK < SP) + Perl_croak_no_modify(); + if (SvCANEXISTDELETE(hv)) can_preserve = TRUE; } @@ -5325,6 +5334,9 @@ PP(pp_hslice) DIE(aTHX_ PL_no_helem_sv, SVfARG(keysv)); } if (localizing) { + if (SvREADONLY(*svp)) + Perl_croak_no_modify(); + if (HvNAME_get(hv) && isGV_or_RVCV(*svp)) save_gp(MUTABLE_GV(*svp), !(PL_op->op_flags & OPf_SPECIAL)); else if (preeminent) -- 2.11.0 ```