Use of freed comppad array during clear_yystack()
authorMarcus Holland-Moritz <mhx-perl@gmx.net>
Wed, 8 Apr 2009 07:49:19 +0000 (09:49 +0200)
committerDavid Mitchell <davem@iabyn.com>
Sat, 25 Jul 2009 23:01:38 +0000 (00:01 +0100)
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.

(cherry picked from commit a8ba03fb2070c532259a5d9d434d5b61c757d31d)

pad.h

diff --git a/pad.h b/pad.h
index 10e094e..352a592 100644 (file)
--- 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",       \