develooper Front page | perl.perl5.porters | Postings from May 2015

Re: [perl #124187] null ptr deref -> S_pad_findlex (pad.c:1141

Thread Previous
From:
Dave Mitchell
Date:
May 5, 2015 14:58
Subject:
Re: [perl #124187] null ptr deref -> S_pad_findlex (pad.c:1141
Message ID:
20150505145836.GR4035@iabyn.com
On Mon, May 04, 2015 at 11:51:56PM -0700, Tony Cook via RT wrote:
> On Mon Apr 27 18:03:33 2015, tonyc wrote:
> > On Mon Apr 27 15:21:58 2015, rjbs wrote:
> > > On Tue Apr 21 22:03:33 2015, tonyc wrote:
> > > > The attached prevents the crash.
> > > >
> > > > There maybe a deeper issue where PL_compcv isn't being restored
> > > > correctly.
> > > 
> > > Are we (read: you) comfortable with this patch as the way to sort this
> > > out?  Is your concern that this will paper over one symptom but leave
> > > a deeper problem still ready to spring, or is that an unrelated
> > > observation?
> > 
> > Yes, that's my concern.
> > 
> > I haven't worked with the parser enough to know whether PL_compcv == NULL
> > is a normal case.
> 
> These also crash in S_pad_findlex(), with different backtraces:
> 
> qq{@{[{}}*sub{]]}}}=<$foo>
> 
> qq{@{[{}}*sub{]]}}}; foo()
> 
> In all three cases, PP_compcv == PL_main_cv and there's no sensible value to 
> replace into PL_compcv.
> 
> I tried changing the code from 9ffcdca1 to leave PL_compcv if it's PL_main_cv,
> but that resulted in later assertion failures.
> 
> I've pushed my original patch from above to blead, since no-one has objected
> to it.

I think my description in de0885da32 covers all these:

commit de0885da327045eed274d7f8913a58e6de0e0f30
Author:     David Mitchell <davem@iabyn.com>
AuthorDate: Tue May 5 10:44:16 2015 +0100
Commit:     David Mitchell <davem@iabyn.com>
CommitDate: Tue May 5 11:23:36 2015 +0100

    null ptr deref in Perl_cv_forget_slab
    
    RT #124385
    
    Parsing following a syntax error could result in a null ptr dereference.
    
    This commit contains a band-aid that returns from Perl_cv_forget_slab() if
    the cv arg is null; but the real issue is much deeper and needs a more
    general fix at some point.
    
    Basically, both the lexer and the parser use the save stack, and after an
    error, they can get out of sync.
    
    In particular:
    
    1) when handling a double-quoted string, the lexer does an ENTER, saves
    most of its current state on the save stack, then uses the literal string
    as the toke source. When it reaches the end of the string, it LEAVEs,
    restores the lexer state and continues with the main source.
    
    2) Whenever the parser starts a new block or sub scope, it remembers the
    current save stack position, and at end of scope, pops the save stack back
    to that position.
    
    In something like
    
        "@{ sub {]}}  }}}"
    
    the lexer sees a double-quoted string, and saves the current lex state.
    The parser sees the start of a sub, and saves PL_compcv etc. Then a parse
    error occurs. The parser goes into error recovery, discarding tokens until
    it can return to a sane state. The lexer runs out of tokens when toking
    the string, does a LEAVE, and switches back to toking the main source.
    This LEAVE restores both the lexer's and the parser's state; in particular
    the parser gets its old PL_compcv restored, even though the parser hasn't
    finished compiling the current sub. Later, series of '}' tokens coming
    through allows the parser to finish the sub. Since PL_error_count > 0, it
    discards the just-compiled sub and sets PL_compcv to null. Normally the
    LEAVE_SCOPE done just after this would restore PL_compcv to its old value
    (e.g. PL_main_cv) but the stack has already been popped, so PL_compcv gets
    left null, and SEGVs occur.
    
    The two main ways I can think of fixing this in the long term are
    1) avoid the lexer using the save stack for long-term state storage;
    in particular, make S_sublex_push() malloc a new parser object rather
    than saving the current lexer state on the save stack.
    2) At the end of a sublex, if PL_error_count > 0, don't try to restore
    state and continue, instead just croak.
    
    N.B. the test that this commit adds to lex.t doesn't actually trigger the
    SEGV, since the bad code is wrapped in an eval which (for reasons I
    haven't researched) avoids the SEGV.


-- 
This is a great day for France!
    -- Nixon at Charles De Gaulle's funeral

Thread Previous


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About