Re: Data overrun in Perl_magic_get '?' (Was: Access violation in SV.C new_body_inline())

[John E. Malmberg]

Craig A. Berry wrote:


On May 18, 2009, at 11:54 PM, John E. Malmberg wrote:


John E. Malmberg wrote:

The trail has taken me to the following lines just below the comment 
with the tag AMS 20010810.


while(mg) {
   const MGVTBL * const vtbl = mg-mg_virtual;

   if (!(mg-mg_flags  MGf_GSKIP)  vtbl  vtbl-svt_get) {
  CALL_FTPR(vtbl-sv_get)(aTHX_ sv, mg);

On the return from this call, the Ibody_roots[7] is corrupted.

*vtbl-sv_get at this point is loaded with the function Perl_magic_get, 
and the value it is looking for is the '?'.


And looking there, I find VMS specific code where the 
my_perl-Istatusvalue_vms is put into sv-sv_any-slv_targlen.


Istatusvalue_vms contains 44, the same bad pointer value.

The body size for type SVt_PVMG (7) appears to be 32, which means that 
the LvTARGLEN(sv) writing at offset 32 is the culprit, corrupting the 
linked list.


I am not sure how to fix this, but now that I have found out this 
much, maybe someone else here can?




So you're saying that these lines in Perl_magic_get in mg.c:

case '?':
{
sv_setiv(sv, (IV)STATUS_CURRENT);
#ifdef COMPLEX_STATUS
LvTARGOFF(sv) = PL_statusvalue;
LvTARGLEN(sv) = PL_statusvalue_vms;
#endif
}


are where the damage occurs?  So it looks like the SV in question does 
not even have the relevant slots (xlv_targlen) we're trying to update 
here.


Yes.

 I wonder if it's because IPC::Cmd declares $? as local.  Maybe we 
are assuming $? is always lexical but it's not?


I do not know.

-John
wb8...@qsl.net
Personal Opinion Only

Re: Data overrun in Perl_magic_get '?' (Was: Access violation in SV.C new_body_inline())

[Craig A. Berry]

On May 19, 2009, at 8:31 AM, Nicholas Clark wrote:


On Tue, May 19, 2009 at 08:18:17AM -0500, John E. Malmberg wrote:

Craig A. Berry wrote:



So you're saying that these lines in Perl_magic_get in mg.c:

  case '?':
  {
  sv_setiv(sv, (IV)STATUS_CURRENT);
#ifdef COMPLEX_STATUS
  LvTARGOFF(sv) = PL_statusvalue;
  LvTARGLEN(sv) = PL_statusvalue_vms;
#endif
  }


are where the damage occurs?  So it looks like the SV in question  
does
not even have the relevant slots (xlv_targlen) we're trying to  
update

here.


Yes.


I wonder if it's because IPC::Cmd declares $? as local.  Maybe we
are assuming $? is always lexical but it's not?


I do not know.


That code rings a bell. The only thing I can find that I did near it  
was:


http://perl5.git.perl.org/perl.git/commit/35f998ddd1e1665f7d0899ae3e50f9262c59d848

However I had a suspicion that I also did something that restricted  
the upgrade

to the minimal case.


Maybe you were thinking of this:

http://www.nntp.perl.org/group/perl.perl5.changes/2008/09/msg22279.html

?

I don't think that lexical has anything to do with it - I suspect  
that the
bug is because IPC::Cmd localises $?, and the new scalar is created  
like this:


STATIC SV *
S_save_scalar_at(pTHX_ SV **sptr, const U32 flags)
{
   dVAR;
   SV * const osv = *sptr;
   register SV * const sv = *sptr = newSV(0);

   PERL_ARGS_ASSERT_SAVE_SCALAR_AT;

   if (SvTYPE(osv) = SVt_PVMG  SvMAGIC(osv)  SvTYPE(osv) !=  
SVt_PVGV) {

if (SvGMAGICAL(osv)) {
const bool oldtainted = PL_tainted;
SvFLAGS(osv) |= (SvFLAGS(osv) 
   (SVp_IOK|SVp_NOK|SVp_POK))  PRIVSHIFT;
PL_tainted = oldtainted;
}
mg_localize(osv, sv, (flags  SAVEf_SETMAGIC) != 0);
   }
   return sv;
}


where that call to mg_localize() will upgrade it to PVMG, but not  
PVLV.


If this is the cause, I'm not sure whether the correct fix is to make
mg_localize generally upgrade the new scalar to the type of the  
existing

scalar, or special case it for $?.



Thanks for the analysis.  I don't really know the implications of the  
alternatives you propose.  I assume a general change is more risky and  
potentially adds unnecessary processing to hot code.  To me the least  
risky thing would be to add the following by analogy with what's  
already going on in gv.c:


--- mg.c;-0 2009-04-27 02:42:10 -0500
+++ mg.c2009-05-20 19:11:11 -0500
@@ -974,6 +974,7 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
{
sv_setiv(sv, (IV)STATUS_CURRENT);
 #ifdef COMPLEX_STATUS
+   SvUPGRADE(sv, SVt_PVLV);
LvTARGOFF(sv) = PL_statusvalue;
LvTARGLEN(sv) = PL_statusvalue_vms;
 #endif
[end]

Does that make sense?

This does fix the memory error that started this thread.  I'm now  
starting a complete test run to make sure nothing else goes pear-shaped.



Craig A. Berry
mailto:craigbe...@mac.com

... getting out of a sonnet is much more
 difficult than getting in.
 Brad Leithauser