Skip to content

Commit

Permalink
fix leak in Perl_vload_module()
Browse files Browse the repository at this point in the history
This function allocates a few ops, then calls Perl_utilize().
If the latter function croaks early on, those ops will be leaked,
because they won't yet have been linked into the optree.

In particular, newUNOP(OP_REQUIRE, ...) can die if passed a non-valid
module name.

This can be fixed by moving the start_subparse() call to the start of
Perl_vload_module(), before any op allocations. start_subparse() creates
a new PL_compcv, and so any ops allocated afterwards will come from that
CV's slab rather than being directly malloc()ed. On death, the CV will
be freed and its op slab will be scanned and any ops found there freed.

The leak was showing up in ext/XS-APItest/t/load-module.t under ASan.
  • Loading branch information
iabyn committed Apr 4, 2019
1 parent 05d49a9 commit 281cff2
Showing 1 changed file with 21 additions and 13 deletions.
34 changes: 21 additions & 13 deletions op.c
Original file line number Diff line number Diff line change
Expand Up @@ -7706,10 +7706,29 @@ void
Perl_vload_module(pTHX_ U32 flags, SV *name, SV *ver, va_list *args)
{
OP *veop, *imop;
OP * const modname = newSVOP(OP_CONST, 0, name);
OP * modname;
I32 floor;

PERL_ARGS_ASSERT_VLOAD_MODULE;

/* utilize() fakes up a BEGIN { require ..; import ... }, so make sure
* that it has a PL_parser to play with while doing that, and also
* that it doesn't mess with any existing parser, by creating a tmp
* new parser with lex_start(). This won't actually be used for much,
* since pp_require() will create another parser for the real work.
* The ENTER/LEAVE pair protect callers from any side effects of use.
*
* start_subparse() creates a new PL_compcv. This means that any ops
* allocated below will be allocated from that CV's op slab, and so
* will be automatically freed if the utilise() fails
*/

ENTER;
SAVEVPTR(PL_curcop);
lex_start(NULL, NULL, LEX_START_SAME_FILTER);
floor = start_subparse(FALSE, 0);

modname = newSVOP(OP_CONST, 0, name);
modname->op_private |= OPpCONST_BARE;
if (ver) {
veop = newSVOP(OP_CONST, 0, ver);
Expand All @@ -7732,18 +7751,7 @@ Perl_vload_module(pTHX_ U32 flags, SV *name, SV *ver, va_list *args)
}
}

/* utilize() fakes up a BEGIN { require ..; import ... }, so make sure
* that it has a PL_parser to play with while doing that, and also
* that it doesn't mess with any existing parser, by creating a tmp
* new parser with lex_start(). This won't actually be used for much,
* since pp_require() will create another parser for the real work.
* The ENTER/LEAVE pair protect callers from any side effects of use. */

ENTER;
SAVEVPTR(PL_curcop);
lex_start(NULL, NULL, LEX_START_SAME_FILTER);
utilize(!(flags & PERL_LOADMOD_DENY), start_subparse(FALSE, 0),
veop, modname, imop);
utilize(!(flags & PERL_LOADMOD_DENY), floor, veop, modname, imop);
LEAVE;
}

Expand Down

0 comments on commit 281cff2

Please sign in to comment.