Clean up PL_strtab hash with NULL in perl_destruct #23346

antirs · 2025-05-31T19:32:50Z

As it's not required to pre-allocate PL_strtab hash before perl_construct() and PL_strtab is not initialized with NULL by default, SIGSEGV could happen in certain circumstances.

Particularly, when application links libperl and compiled with ASAN. PL_strtab remains unitialized as at start-up it contains garbage and initialization in perl_construct() is skipped because !PL_strtab check is skipped.

See also: c82f488 (Allow custom PL_strtab hash in perl_construct.)

Crash log may look like this:

ASAN_OPTIONS=report_globals=0:detect_leaks=1:handle_segv=1:handle_sigbus=1:handle_abort=1 ./fuzz -jobs=1 -runs=1 -seed=666 </dev/null
./fuzz -runs=1 -seed=666 >fuzz-0.log 2>&1
================== Job 0 exited with exit code 1 ============
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 666
INFO: A corpus is not provided, starting from an empty corpus
AddressSanitizer:DEADLYSIGNAL
=================================================================
==22328==ERROR: AddressSanitizer: SEGV on unknown address 0x000000081880 (pc 0x5599c1528155 bp 0x7ffe87a38d70 sp 0x7ffe87a38d00 T0)
==22328==The signal is caused by a READ memory access.
    #0 0x5599c1528155 in S_share_hek_flags /mnt/downloads/temp/_code/PERL/perl5/hv.c:3426:11
    #1 0x5599c15280ce in Perl_share_hek /mnt/downloads/temp/_code/PERL/perl5/hv.c:3399:12
    #2 0x5599c168db46 in Perl_newSVpvn_share /mnt/downloads/temp/_code/PERL/perl5/sv.c:9923:5
    #3 0x5599c14eca43 in S_init_main_stash /mnt/downloads/temp/_code/PERL/perl5/perl.c:4140:20
    #4 0x5599c14e88a0 in S_parse_body /mnt/downloads/temp/_code/PERL/perl5/perl.c:2223:5
    #5 0x5599c14e81b1 in perl_parse /mnt/downloads/temp/_code/PERL/perl5/perl.c:1932:9
...

This set of changes does not require a perldelta entry.

jkeenan · 2025-05-31T22:21:08Z

Since your pull request is intended to correct a possible segfault, can you supply the output of perl -V for the perl build where you observe the problem? Thanks.

antirs · 2025-06-01T06:06:36Z

The output of `perl -V`:

Summary of my perl5 (revision 5 version 41 subversion 14) configuration:
  Derived from: f9d1ecdb4e3a32ee002d062294dc02e4ce5fdab0
  Platform:
    osname=linux
    osvers=6.12.27-amd64
    archname=x86_64-linux
    uname='linux substation 6.12.27-amd64 #1 smp preempt_dynamic debian 6.12.27-1 (2025-05-06) x86_64 gnulinux '
    config_args='-des -Doptimize=-g -Dusedevel -DDEBUGGING=-g -Uusenm -Dprefix=/mnt/downloads/temp/_code/PERL/perl5/.install'
    hint=previous
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-g'
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='14.2.0'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /usr/lib64
    libs=-lpthread -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/lib/x86_64-linux-gnu/libc.so.6
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.41'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong'


Characteristics of this binary (from libperl): 
  Compile-time options:
    HAS_LONG_DOUBLE
    HAS_STRTOLD
    HAS_TIMES
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_HASH_FUNC_SIPHASH13
    PERL_HASH_USE_SBOX32
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_USE_DEVEL
    PERL_USE_SAFE_PUTENV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
  Locally applied patches:
    uncommitted-changes
  Built under linux
  Compiled at Jun  1 2025 08:54:50
  %ENV:
    PERL5LIB="/home/enomem/.perl5/lib/perl5:/home/enomem/.perl5/lib/perl5/x86_64-linux/auto:/home/enomem/.perl5/lib/perl5:/home/enomem/.perl5/lib/perl5/x86_64-linux/auto:/home/enomem/.perl5/lib/perl5:/home/enomem/.perl5/lib/perl5/x86_64-linux/auto"
    PERL_LOCAL_LIB_ROOT="/home/enomem/.perl5:/home/enomem/.perl5:/home/enomem/.perl5"
    PERL_MB_OPT="--install_base "/home/enomem/.perl5""
    PERL_MM_OPT="INSTALL_BASE=/home/enomem/.perl5"
  @INC:
    /home/enomem/.perl5/lib/perl5/x86_64-linux
    /home/enomem/.perl5/lib/perl5
    /home/enomem/.perl5/lib/perl5/x86_64-linux/auto
    /home/enomem/.perl5/lib/perl5/x86_64-linux
    /home/enomem/.perl5/lib/perl5
    /home/enomem/.perl5/lib/perl5/x86_64-linux/auto
    /home/enomem/.perl5/lib/perl5/x86_64-linux
    /home/enomem/.perl5/lib/perl5
    /home/enomem/.perl5/lib/perl5/x86_64-linux/auto
    /mnt/downloads/temp/_code/PERL/perl5/.install/lib/site_perl/5.41.14/x86_64-linux
    /mnt/downloads/temp/_code/PERL/perl5/.install/lib/site_perl/5.41.14
    /mnt/downloads/temp/_code/PERL/perl5/.install/lib/5.41.14/x86_64-linux
    /mnt/downloads/temp/_code/PERL/perl5/.install/lib/5.41.14

ap · 2025-06-06T15:17:48Z

@tonycoz @iabyn @Leont @leonerd or anyone else who understands this – can you opine? If possible we want to ship this in this cycle.

Leont

Yeah this makes sense to me.

Ironically I think it's only a problem on non-multiplicity perls (on multiplicity the perl struct is allocated and null initialized.

tonycoz · 2025-06-10T00:27:37Z

For non-MULTIPLICITY builds PL_strtab is a statically allocated global, without an initializer it should be initialized to NULL by the C runtime/compiler/linker etc (ie. as required by the C language)

I suspect the real fix here is to zero PL_strtab when it is released in perl_destruct.

antirs · 2025-06-10T09:01:15Z

Crash happens when perl is compiled with libFuzzer in a program like this:

fuzz.c

#include "EXTERN.h"
#include "perl.h"
#include "XSUB.h"

#include "ppport.h"

#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

static PerlInterpreter *my_perl;


int FuzzMe(const char *data, size_t size) {
    int exitstatus, i;
    char *cmd[] = {"", "-e 0", NULL};
    my_perl = perl_alloc();
    perl_construct(my_perl);
    PL_perl_destruct_level = 1;
    if (!perl_parse(my_perl, NULL, 2, cmd, (char **)NULL)) {
        perl_run(my_perl);
    }

    exitstatus = perl_destruct(my_perl);
    perl_free(my_perl);
    return 0;
}

extern int LLVMFuzzerTestOneInput(const char *data, size_t size) {
    FuzzMe(data, size);
    return 0;
}

Compiled with

compilation commands

clang -ggdb -I. -fwrapv -g -DDEBUGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -DDEBUGGING -g   -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC -I/mnt/downloads/temp/_code/PERL/perl5/.install/lib/5.41.14/x86_64-linux/CORE   -c -o fuzz.o fuzz.c
clang -ggdb -I. -fwrapv -g -DDEBUGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -DDEBUGGING -g   -DVERSION=\"0.01\" -DXS_VERSION=\"0.01\" -fPIC -I/mnt/downloads/temp/_code/PERL/perl5/.install/lib/5.41.14/x86_64-linux/CORE -fsanitize=address,fuzzer -fprofile-instr-generate -fcoverage-mapping -o fuzz fuzz.o -L /mnt/downloads/temp/_code/PERL/perl5/ -lperl -lcrypt

When pre-initialized with NULL, there is no crash.
Without ASAN it runs normally.

jkeenan · 2025-06-10T22:18:04Z

If possible we want to ship this in this cycle.

Emphasis added and marking ticket "Release Blocker."

tonycoz · 2025-06-11T04:01:48Z

When pre-initialized with NULL, there is no crash.
Without ASAN it runs normally.

Yes, you're running the interpreter multiple times.

The first time around PL_strtab is NULL, so perl_construct() creates a HV, and then perl_destruct() frees it, but doesn't clear the global PL_strtab variable.

The second time around PL_strtab points at the now freed HV, and so isn't initialized to point to a new HV, and the interpreter proceeds to use the bad pointer and dies horribly.

If I test with code like:

int RunIt(const char *data, size_t size) {
    int exitstatus, i;
    char *cmd[] = {"", "-e 0", NULL};
    my_perl = perl_alloc();
    perl_construct(my_perl);
    PL_perl_destruct_level = 1;
    if (!perl_parse(my_perl, NULL, 2, cmd, (char **)NULL)) {
        perl_run(my_perl);
    }

    exitstatus = perl_destruct(my_perl);
    perl_free(my_perl);
    return 0;
}

int main(int argc, char **argv) {
    RunIt("one", 3);
    RunIt("two", 3);
}

I get the same type of crash you did without ASAN or a fuzzer:

==206276== Invalid read of size 8
==206276==    at 0x1B7110: S_share_hek_flags (hv.c:3407)
==206276==    by 0x1B70D3: Perl_share_hek (hv.c:3399)
==206276==    by 0x42AD9A: Perl_newSVpvn_share (sv.c:9923)
==206276==    by 0x14D2E0: S_init_main_stash (perl.c:4140)
==206276==    by 0x145B3C: S_parse_body (perl.c:2223)
==206276==    by 0x145075: perl_parse (perl.c:1932)
==206276==    by 0x13F0AF: RunIt (embedtwice.c:20)
==206276==    by 0x13F135: main (embedtwice.c:31)
==206276==  Address 0x4b783c8 is 1,512 bytes inside a block of size 4,080 free'd
==206276==    at 0x484417B: free (vg_replace_malloc.c:872)
==206276==    by 0x4A4C15: Perl_safesysfree (util.c:433)
==206276==    by 0x3E7115: Perl_sv_free_arenas (sv.c:713)
==206276==    by 0x143F33: perl_destruct (perl.c:1533)
==206276==    by 0x13F0D1: RunIt (embedtwice.c:24)
==206276==    by 0x13F121: main (embedtwice.c:30)
==206276==  Block was alloc'd at
==206276==    at 0x48417B4: malloc (vg_replace_malloc.c:381)
==206276==    by 0x4A48B9: Perl_safesysmalloc (util.c:176)
==206276==    by 0x3E51F8: Perl_more_sv (sv.c:277)
==206276==    by 0x3E292E: Perl_new_sv (sv_inline.h:85)
==206276==    by 0x3E2A19: Perl_newSV_type (sv_inline.h:378)
==206276==    by 0x42A3B7: Perl_newSVpvn (sv.c:9792)
==206276==    by 0x141C4D: perl_construct (perl.c:261)
==206276==    by 0x13F082: RunIt (embedtwice.c:18)
==206276==    by 0x13F121: main (embedtwice.c:30)

(I get a Segmentation fault without valgrind too.)

If I clear PL_strtab after releasing it in perl_destruct() it no longer crashes.

Adding an initializer to the strtab as you did means that if the caller sets PL_strtab to a custom HV as licensed by c82f488, PL_strtab will be cleared by S_init_interp() before the later code in perl_construct() gets a chance to use it. This will also leak the custom HV.

tonycoz · 2025-06-11T05:52:51Z

It could be we don't want to support pre-setting PL_strtab as c82f488 enables, if we want that we can just revert c82f488 and PL_strtab will be set unconditionally.

With this PR the if (!PL_strtab) { is always true.

antirs · 2025-06-11T06:30:26Z

Finally, I've got it.
Now commit message is unrelated, as initial fix in PR was about initializing PL_strtab, but now it's for cleaning.

ap · 2025-06-12T15:35:09Z

This has been around for much longer than just this release. That doesn’t meant we wouldn’t very much like to ship a fix in this release… once it becomes clear what the right fix is.

tonycoz · 2025-06-12T22:41:16Z

I think the current change is ok, though using github to merge blead (I assume from the email address and history) broke the authors porting test.

@antirs, please rebase on blead and squash your commits, this should remove the merge and make it ready to apply.

As it's not required to pre-allocate PL_strtab hash before perl_construct() and PL_strtab is not initialized with NULL by default, SIGSEGV could happen in certain circumstances. Particularly, when application links libperl and runs perl several times. After the first run PL_strtab remains unitialized in perl_construct() as it was not cleared in perl_destruct() and it contains garbage and the next initialization in perl_construct() is skipped because !PL_strtab check is skipped. See also: c82f488 (Allow custom PL_strtab hash in perl_construct.)

Leont approved these changes Jun 6, 2025

View reviewed changes

jkeenan added the Release Blocker label Jun 10, 2025

antirs force-pushed the dev/enomem/fix-crashes-on-unitialized-string-hash branch from a1c407c to 3d7f78a Compare June 11, 2025 06:26

antirs force-pushed the dev/enomem/fix-crashes-on-unitialized-string-hash branch from 3d7f78a to 0ad853a Compare June 11, 2025 09:02

antirs changed the title ~~Initialize PL_strtab hash with NULL by default~~ Clean up PL_strtab hash with NULL in perl_destruct Jun 11, 2025

ap removed the Release Blocker label Jun 12, 2025

antirs force-pushed the dev/enomem/fix-crashes-on-unitialized-string-hash branch 2 times, most recently from fa688c7 to 2345cf0 Compare June 13, 2025 05:43

antirs force-pushed the dev/enomem/fix-crashes-on-unitialized-string-hash branch from 2345cf0 to 87c0504 Compare June 13, 2025 05:46

tonycoz approved these changes Jun 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Clean up PL_strtab hash with NULL in perl_destruct #23346

Clean up PL_strtab hash with NULL in perl_destruct #23346

Uh oh!

antirs commented May 31, 2025

Uh oh!

jkeenan commented May 31, 2025

Uh oh!

antirs commented Jun 1, 2025

Uh oh!

ap commented Jun 6, 2025

Uh oh!

Leont left a comment

Uh oh!

tonycoz commented Jun 10, 2025

Uh oh!

antirs commented Jun 10, 2025 •

edited

Loading

Uh oh!

jkeenan commented Jun 10, 2025

Uh oh!

tonycoz commented Jun 11, 2025

Uh oh!

tonycoz commented Jun 11, 2025

Uh oh!

antirs commented Jun 11, 2025

Uh oh!

ap commented Jun 12, 2025

Uh oh!

tonycoz commented Jun 12, 2025

Uh oh!

Uh oh!

Clean up PL_strtab hash with NULL in perl_destruct #23346

Are you sure you want to change the base?

Clean up PL_strtab hash with NULL in perl_destruct #23346

Uh oh!

Conversation

antirs commented May 31, 2025

Uh oh!

jkeenan commented May 31, 2025

Uh oh!

antirs commented Jun 1, 2025

Uh oh!

ap commented Jun 6, 2025

Uh oh!

Leont left a comment

Choose a reason for hiding this comment

Uh oh!

tonycoz commented Jun 10, 2025

Uh oh!

antirs commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkeenan commented Jun 10, 2025

Uh oh!

tonycoz commented Jun 11, 2025

Uh oh!

tonycoz commented Jun 11, 2025

Uh oh!

antirs commented Jun 11, 2025

Uh oh!

ap commented Jun 12, 2025

Uh oh!

tonycoz commented Jun 12, 2025

Uh oh!

Uh oh!

antirs commented Jun 10, 2025 •

edited

Loading