From: Marcus Holland-Moritz <mhx-perl@gmx.net>
Date: Wed, 8 Apr 2009 07:49:19 +0000 (+0200)
Subject: Use of freed comppad array during clear_yystack()
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=a8ba03fb2070c532259a5d9d434d5b61c757d31d;p=p5sagit%2Fp5-mst-13.2.git

Use of freed comppad array during clear_yystack()

Message-ID: <20081026231720.34258457@r2d2>

Patch description from the original email :

I tried to make tests pass on a perl built with -DPERL_POISON,
as some tests were dying with segfaults. They all originated
from the same source: clear_yystack() after a compile error. [...]
As far as I can see, after croaking the newly
created CV is destroyed and its pad is undef'd. [...]

This will SvREFCNT_dec PL_comppad and set PL_comppad to NULL.
However, later, in clear_yystack(), when the ops are freed, the
old PL_comppad is restored by PAD_RESTORE_LOCAL, as a reference
is still in ps->comppad. But now the pad AV is already dead.

Normally (i.e. without PERL_POISON), the dead AV will have
AvARRAY(av) set to NULL by av_undef(). So PAD_RESTORE_LOCAL will
actually set PL_curpad to NULL, and thus pad_free() will not
attempt to do anything.

But with PERL_POISON, the storage for AvARRAY(av) (i.e. sv_u)
will be reused for chaining the free SV heads in the arena
(as opposed to SvANY(sv) in case of !PERL_POISON). This means
that PAD_RESTORE_LOCAL will find AvARRAY(av) non-NULL and will
set PL_curpad to that value, finally causing the segfault in
pad_free().

While I think I understand what's going on, I don't have the
slightest clue how to properly fix this. Given that it's not
a problem only under PERL_POISON, but always (as dead SV heads
are being used), I think it should ultimately be fixed.

The only thing I can offer right now is a patch to make it
work with PERL_POISON as good (or as bad) as without by
making PAD_RESTORE_LOCAL explicitly check if the pad passed
in is already dead and refusing to use it if it is.
---

diff --git a/pad.h b/pad.h
index 10e094e..352a592 100644
--- a/pad.h
+++ b/pad.h
@@ -239,7 +239,7 @@ Restore the old pad saved into the local variable opad by PAD_SAVE_LOCAL()
 	      PTR2UV(PL_comppad), PTR2UV(PL_curpad)));
 
 #define PAD_RESTORE_LOCAL(opad) \
-	PL_comppad = opad;					\
+	PL_comppad = opad && SvIS_FREED(opad) ? NULL : opad;	\
 	PL_curpad =  PL_comppad ? AvARRAY(PL_comppad) : NULL;	\
 	DEBUG_Xv(PerlIO_printf(Perl_debug_log,			\
 	      "Pad 0x%"UVxf"[0x%"UVxf"] restore_local\n",	\