Stack overflow when local'izing readonly arrays #16954

p5pRT · 2019-04-14T10:03:11Z

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

Searchable as RT134028$

p5pRT · 2019-04-14T10:03:11Z

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 · 2019-04-23T01:19:56Z

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 · 2019-04-23T01:19:56Z

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

p5pRT · 2019-04-23T10:17:36Z

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 · 2019-04-23T10:17:36Z

From @tonycoz

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

From f27364453c2e45a51516c6705bcc61277c6faa78 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
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

p5pRT added Severity Low type-core labels Oct 19, 2019

xenu removed the affects-5.29 label Nov 19, 2021

xenu removed the Severity Low label Dec 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stack overflow when local'izing readonly arrays #16954

Stack overflow when local'izing readonly arrays #16954

p5pRT commented Apr 14, 2019

p5pRT commented Apr 14, 2019

p5pRT commented Apr 23, 2019

p5pRT commented Apr 23, 2019

p5pRT commented Apr 23, 2019

p5pRT commented Apr 23, 2019

Stack overflow when local'izing readonly arrays #16954

Stack overflow when local'izing readonly arrays #16954

Comments

p5pRT commented Apr 14, 2019

p5pRT commented Apr 14, 2019

From @dur-randir

Created by @dur-randir

p5pRT commented Apr 23, 2019

From @tonycoz

p5pRT commented Apr 23, 2019

p5pRT commented Apr 23, 2019

From @tonycoz

p5pRT commented Apr 23, 2019

From @tonycoz