stack pointer corruption in pp_concat() with 'use encoding'

[p5pRT]

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

Searchable as RT78674$

[p5pRT]

From @ntyni

This is a bug report for perl from Niko Tyni <ntyni@​debian.org>,
generated with the help of perlbug 1.39 running under perl 5.13.6.

---

./perl -Ilib -Mencoding=utf8 -e 'map { "a" . $a } ((1)x500);'
panic​: bad free during global destruction.

Valgrind shows errors like

==25202== Invalid write of size 8
==25202== at 0x5714BD​: Perl_pp_concat (pp_hot.c​:289)
==25202== by 0x52774B​: Perl_runops_debug (dump.c​:2120)
==25202== by 0x453032​: S_run_body (perl.c​:2314)
==25202== by 0x45225A​: perl_run (perl.c​:2238)
==25202== by 0x41DACC​: main (perlmain.c​:117)
==25202== Address 0x61572b0 is 4,064 bytes inside a block of size 4,104 free'd
==25202== at 0x4C240FD​: free (vg_replace_malloc.c​:366)
==25202== by 0x5287CD​: Perl_safesysfree (util.c​:280)
==25202== by 0x5678CD​: Perl_av_extend (av.c​:153)
==25202== by 0x646159​: Perl_stack_grow (scope.c​:38)
==25202== by 0x45401E​: Perl_call_sv (perl.c​:2570)
==25202== by 0x453DF1​: Perl_call_method (perl.c​:2522)
==25202== by 0xCA493B4​: XS_Encode__utf8_decode_xs (Encode.xs​:446)
==25202== by 0x58E063​: Perl_pp_entersub (pp_hot.c​:2945)
==25202== by 0x52774B​: Perl_runops_debug (dump.c​:2120)
==25202== by 0x45438A​: Perl_call_sv (perl.c​:2596)
==25202== by 0x453DF1​: Perl_call_method (perl.c​:2522)
==25202== by 0x5FB7E4​: Perl_sv_recode_to_utf8 (sv.c​:13308)
==25202==

This is due to stack pointer corruption in Perl_pp_concat() when the
stack gets reallocated in the sv_utf8_upgrade_nomg() call (implemented
with sv_utf8_upgrade_flags_grow()). See below for the full backtrace to
the corresponding Perl_stack_grow() call.

Proposed patch attached. Sorry, couldn't figure how to write a regression
test for this.

It seems possible that there are other places where
Perl_sv_utf8_upgrade_flags_grow() gets called with a local copy of the
stack pointer without the PUTBACK/SPAGAIN guards. I haven't looked for
these systematically.

Originally reported by Ken Bloom in http​://bugs.debian.org/596105

#0 Perl_stack_grow (my_perl=0xa3f010, sp=0xb456d0, p=0xb456d0, n=1) at scope.c​:34
#1 0x000000000045401f in Perl_call_sv (my_perl=0xa3f010, sv=0xa620a8, flags=130) at perl.c​:2570
#2 0x0000000000453df2 in Perl_call_method (my_perl=0xa3f010, methname=0x7ffff03c324f "renewed", flags=2)
  at perl.c​:2522
#3 0x00007ffff03b63b5 in XS_Encode__utf8_decode_xs (my_perl=0xa3f010, cv=0xb3b950) at Encode.xs​:446
#4 0x000000000058e064 in Perl_pp_entersub (my_perl=0xa3f010) at pp_hot.c​:2945
#5 0x000000000052774c in Perl_runops_debug (my_perl=0xa3f010) at dump.c​:2120
#6 0x000000000045438b in Perl_call_sv (my_perl=0xa3f010, sv=0xa42c30, flags=130) at perl.c​:2596
#7 0x0000000000453df2 in Perl_call_method (my_perl=0xa3f010, methname=0x7ddca0 "decode", flags=2)
  at perl.c​:2522
#8 0x00000000005fb7e5 in Perl_sv_recode_to_utf8 (my_perl=0xa3f010, sv=0xa429f0, encoding=0xa62bd0)
  at sv.c​:13308
#9 0x00000000005adabe in Perl_sv_utf8_upgrade_flags_grow (my_perl=0xa3f010, sv=0xa429f0, flags=0,
  extra=0) at sv.c​:3215
#10 0x00000000005712d6 in Perl_pp_concat (my_perl=0xa3f010) at pp_hot.c​:283
#11 0x000000000052774c in Perl_runops_debug (my_perl=0xa3f010) at dump.c​:2120
#12 0x0000000000453033 in S_run_body (my_perl=0xa3f010, oldscope=1) at perl.c​:2314
#13 0x000000000045225b in perl_run (my_perl=0xa3f010) at perl.c​:2238
#14 0x000000000041dacd in main (argc=5, argv=0x7fffffffe8f8, env=0x7fffffffe928) at perlmain.c​:117

---

---

Flags​:
  category=core
  severity=medium

---

Site configuration information for perl 5.13.6​:

Configured by niko at Thu Oct 28 23​:42​:17 EEST 2010.

Summary of my perl5 (revision 5 version 13 subversion 6) configuration​:
  Derived from​: 6911354
  Platform​:
  osname=linux, osvers=2.6.32-5-amd64, archname=x86_64-linux-gnu-thread-multi
  uname='linux madeleine 2.6.32-5-amd64 #1 smp wed oct 20 00​:05​:22 utc 2010 x86_64 gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.13 -Darchlib=/usr/lib/perl/5.13 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.13.6 -Dsitearch=/usr/local/lib/perl/5.13.6 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=both -Doptimize=-O0 -Dusedevel -Uuseshrplib -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O0 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.4.5', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
  libc=/lib/libc-2.11.2.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.11.2'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O0 -g -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 

---

@​INC for perl 5.13.6​:
  lib
  /usr/local/lib/perl/5.13.6
  /usr/local/share/perl/5.13.6
  /usr/lib/perl5
  /usr/share/perl5
  /usr/lib/perl/5.13
  /usr/share/perl/5.13
  /usr/local/share/perl
  /usr/share/perl5
  .

---

Environment for perl 5.13.6​:
  HOME=/home/niko
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LC_CTYPE=fi_FI.UTF-8
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/home/niko/bin​:/home/niko/bin​:/home/niko/bin​:/usr/local/bin​:/usr/bin​:/bin​:/usr/local/games​:/usr/games​:/sbin​:/usr/sbin​:/sbin​:/usr/sbin
  PERL_BADLANG (unset)
  SHELL=/bin/zsh

[p5pRT]

From @ntyni

0001-Fix-stack-pointer-corruption-in-pp_concat-with-use-e.patch

From 286215bec6f59ddd4f27fc5c21c76e3d762ed771 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Thu, 28 Oct 2010 23:52:17 +0300
Subject: [PATCH] Fix stack pointer corruption in pp_concat() with 'use encoding'

sv_utf8_upgrade_nomg() may reallocate the stack via sv_recode_to_utf8()
if 'use encoding' is in effect, causing stack pointer corruption.
---
 pp_hot.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/pp_hot.c b/pp_hot.c
index fd270e9..f4d79dc 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -275,6 +275,8 @@ PP(pp_concat)
 	rbyte = !DO_UTF8(right);
     }
     if (lbyte != rbyte) {
+	/* sv_utf8_upgrade_nomg() may reallocate the stack */
+	PUTBACK;
 	if (lbyte)
 	    sv_utf8_upgrade_nomg(TARG);
 	else {
@@ -283,6 +285,7 @@ PP(pp_concat)
 	    sv_utf8_upgrade_nomg(right);
 	    rpv = SvPV_nomg_const(right, rlen);
 	}
+	SPAGAIN;
     }
     sv_catpvn_nomg(TARG, rpv, rlen);
 
-- 
1.7.2.3

[p5pRT]

From @cpansprout

On Thu Oct 28 14​:22​:36 2010, ntyni@​debian.org wrote​:

Proposed patch attached.

Thank you. Applied as e3393f5.

Sorry, couldn't figure how to write a
regression
test for this.

Like this​: http​://perl5.git.perl.org/perl.git/commitdiff/1222f39eae03eee

[p5pRT]

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

[p5pRT]

@cpansprout - Status changed from 'open' to 'resolved'

[p5pRT]

From @nwc10

On Thu, Oct 28, 2010 at 02​:22​:36PM -0700, Niko Tyni wrote​:

This is due to stack pointer corruption in Perl_pp_concat() when the
stack gets reallocated in the sv_utf8_upgrade_nomg() call (implemented
with sv_utf8_upgrade_flags_grow()). See below for the full backtrace to
the corresponding Perl_stack_grow() call.

I can't find any.

I was wondering, is a better solution to this whole problem to ensure that
anything that creates an inferior runloop does so on a freshly allocated
stack?

The whole paradigm of needing to get and "put back" a local copy of the
stack pointer seems fragile, and something that dates from an age of machines
with much tighter resource constraints.

(Heck, and effectively predates common use of tie, overloading, PerlIO and
the growing supply of hooks which can end up running code within what looks
like an innocent C function call)

Nicholas Clark

[p5pRT]

From @Leont

On Wed, May 8, 2013 at 2​:18 PM, Nicholas Clark <nick@​ccl4.org> wrote​:

I can't find any.

I was wondering, is a better solution to this whole problem to ensure that
anything that creates an inferior runloop does so on a freshly allocated
stack?

A number of magical thingies already do such a thing, it does make
sense to me given circumstances.

The whole paradigm of needing to get and "put back" a local copy of the
stack pointer seems fragile, and something that dates from an age of machines
with much tighter resource constraints.

Yeah, it's fairly annoying, though AFAIK it can't be made no-op
without subtly breaking backwards compatibility. A simpler stack
discipline may be nice to have though it may be too late for that.
Then again, stack refcounting is probably a bigger and harder issue.

(Heck, and effectively predates common use of tie, overloading, PerlIO and
the growing supply of hooks which can end up running code within what looks
like an innocent C function call)

Yeah. It's an optimization that might not be an optimization anymore.
I mean, the cost of creating new stacks occasionally may be higher
than the gain of mostly working with local variables.

Leon