This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
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)
committerRafael Garcia-Suarez <rgarciasuarez@gmail.com>
Wed, 8 Apr 2009 07:49:19 +0000 (09:49 +0200)
commita8ba03fb2070c532259a5d9d434d5b61c757d31d
treeea8dcc211884b022d4953c3b764d32f9c9a52b78
parent36d7a91d2021ad52ce172e2c75a7bc5fa07e6354
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.
pad.h