This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
fix leak in Perl_vload_module()
authorDavid Mitchell <davem@iabyn.com>
Thu, 4 Apr 2019 13:38:50 +0000 (14:38 +0100)
committerDavid Mitchell <davem@iabyn.com>
Thu, 4 Apr 2019 13:38:50 +0000 (14:38 +0100)
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.

op.c

diff --git a/op.c b/op.c
index 3b0cc76..350032a 100644 (file)
--- a/op.c
+++ b/op.c
@@ -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);
@@ -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;
 }