Skip Menu |
Report information
Id: 132234
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: brian.carpenter [at] gmail.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



From: Brian Carpenter <brian.carpenter [...] gmail.com>
Subject: use-of-uninitialized-value in Perl_upg_version (vutil.c:669)
Date: Fri, 6 Oct 2017 15:42:07 -0500
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
While building 1195d90 with -fsanitize=memory, the process fails during the miniperl build.

afl-clang-fast [tpcg] 2.51b by <lszekeres@google.com>
./miniperl -w -Ilib -Idist/Exporter/lib -MExporter -e '<?>' || sh -c 'echo >&2 Failed to build miniperl.  Please run make minitest; exit 1'
==11441==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x560a10786ab7 in Perl_upg_version /root/perl/./vutil.c:669:2
    #1 0x560a107839a2 in Perl_new_version /root/perl/./vutil.c:551:12
    #2 0x560a10acb9f4 in S_require_version /root/perl/pp_ctl.c:3663:10
    #3 0x560a10acb9f4 in Perl_pp_require /root/perl/pp_ctl.c:4291
    #4 0x560a10757a0d in Perl_runops_debug /root/perl/dump.c:2486:23
    #5 0x560a10432f4e in Perl_call_sv /root/perl/perl.c:2921:6
    #6 0x560a10419c56 in Perl_call_list /root/perl/perl.c:5086:6
    #7 0x560a103b2253 in S_process_special_blocks /root/perl/op.c:9061:6
    #8 0x560a1037a348 in Perl_newATTRSUB_x /root/perl/op.c:8990:21
    #9 0x560a1038434b in Perl_utilize /root/perl/op.c:6341:5
    #10 0x560a105c3ee4 in Perl_yyparse /root/perl/perly.y:360:6
    #11 0x560a104296f9 in S_parse_body /root/perl/perl.c:2450:9
    #12 0x560a1041dc2e in perl_parse /root/perl/perl.c:1753:2
    #13 0x560a10e00597 in main /root/perl/miniperlmain.c:127:18
    #14 0x7fa54f9ea82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #15 0x560a102b4e88 in _start (/root/perl/miniperl+0x2fe88)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /root/perl/./vutil.c:669:2 in Perl_upg_version
Exiting
Failed to build miniperl. Please run make minitest
makefile:362: recipe for target 'lib/buildcustomize.pl' failed
make: *** [lib/buildcustomize.pl] Error 1
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 297b
On Fri, 06 Oct 2017 13:42:55 -0700, brian.carpenter@gmail.com wrote: Show quoted text
> While building 1195d90 with -fsanitize=memory, the process fails during the > miniperl build.
Could I have the Configure options and clang version you used to build this? (since you obviously can't run perl -V) Thanks, Tony
From: Brian Carpenter <brian.carpenter [...] gmail.com>
Subject: Re: [perl #132234] use-of-uninitialized-value in Perl_upg_version (vutil.c:669)
To: perlbug-followup [...] perl.org
Date: Wed, 18 Oct 2017 20:08:06 -0500
Download (untitled) / with headers
text/plain 935b
./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-clang-fast -Doptimize=-O2\ -g -Accflags='-fno-omit-frame-pointer -fsanitize=memory -fsanitize-coverage=edge,trace-pc-guard,indirect-calls,trace-cmp,trace-div,trace-gep' -Aldflags='-fno-omit-frame-pointer -fsanitize=memory -fsanitize-coverage=edge,trace-pc-guard,indirect-calls,trace-cmp,trace-div,trace-gep' &&  AFL_PATH=/root/afl-2.51b make

Chromium clang version 6.0.0-trunk: git clone https://chromium.googlesource.com/chromium/src/tools/clang && clang/scripts/update.py





On Wed, Oct 18, 2017 at 7:51 PM, Tony Cook via RT <perlbug-followup@perl.org> wrote:
Show quoted text
On Fri, 06 Oct 2017 13:42:55 -0700, brian.carpenter@gmail.com wrote:
> While building 1195d90 with -fsanitize=memory, the process fails during the
> miniperl build.

Could I have the Configure options and clang version you used to build this? (since you obviously can't run perl -V)

Thanks,
Tony


From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #132234] use-of-uninitialized-value in Perl_upg_version (vutil.c:669)
Date: Sat, 9 Dec 2017 23:35:47 +0000
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 3.5k
Brian Carpenter wrote: Show quoted text
>==11441==WARNING: MemorySanitizer: use-of-uninitialized-value > #0 0x560a10786ab7 in Perl_upg_version /root/perl/./vutil.c:669:2
I can't make sense of this, but I can see some other flaws in the same code. That bit of code is concerned with expanding an NV to a version object, and more specifically with rendering the NV as a string. The process it's trying to achieve is basically $_ = sprintf("%.9f", $nv); s/\.?0+\z//; Line 669 is part of that trimming operation: while (buf[len-1] == '0' && len > 0) len--; One can immediately see a flaw, that the conditions are in the wrong order. The "len > 0" is clearly concerned about running off the beginning of the buffer, and ought to be tested before reading from buf[len-1]. If the memory immediately before buf counts as uninitialised, but is mapped so accessing it doesn't segv, this would explain getting the correct behaviour from this line while also getting this uninitialised read. But the logic of the situation is such that this trimming operation can't ever reach the beginning of the string. The sprintf format means that the trimmable trailing digits must always be preceded by the decimal point, which itself must be preceded by at least one digit. Indeed, the following line of code if ( buf[len-1] == '.' ) len--; /* eat the trailing decimal */ shows no worry about hitting the beginning of the string. And if I put in assertions that check for len reaching zero, those assertions always pass, not only in building but also in the entire test suite. I don't see how the trimming operation could ever reach the beginning of the string. So the "len > 0" on line 669 isn't so much misplaced as completely extraneous. It should be removed, possibly replaced with assertions. If those lines are OK, what about the preceding code that sets up buf and len? There are two versions of this code, one using a stack-allocated fixed-size buffer, and one using an SV. There's logic attempting to use the fixed-size buffer if the result will fit, with fallback to the SV. The use of "10e50" as a threshold for SV use suggests that the author didn't quite understand "e" notation, but ultimately I believe the threshold is adequate to avoid overrunning the fixed-size buffer. The code that uses the fixed-size buffer goes further to avoid overrunning it, by using my_snprintf() rather than straight sprintf(). This should be unnecessary, given the previous logic. It also doesn't actually avoid undefined behaviour, because len would still get the untruncated string length. So using snprintf() rather than straight sprintf() here isn't achieving much; either way it would be more useful to have an assertion that truncation didn't happen. If I insert such an assertion, it always passes, as we'd expect. The code that uses an SV buffer is much safer, assuming we can rely on the buffer management of the core SV code. However, it has an entertaining bug, in that it constructs the SV initially with an undef value and then uses sv_catpvf() to append the formatted string. So if warnings are enabled in the relevant scope, the version upgrade produces a bogus "uninitialized value" warning. (Try "require(2e51)".) If I change the buffer choice logic to always go the SV route, hundreds of such warnings are duly produced. Correcting sv_catpvf() to sv_setpvf() resolves those warnings, and with that bugfix the always-SV version of the function passes all tests. I believe that code is correct apart from the undef issue. So plenty of flaws there, but I don't see the one claimed by the bug report. -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 397b
On Sat, 09 Dec 2017 15:36:55 -0800, zefram@fysh.org wrote: Show quoted text
> So plenty of flaws there, but I don't see the one claimed by the bug > report.
Whatever changes you might make, please make sure they get submitted to version.pm’s bug queue, and that they are compatible with older perl versions. (Many committers have thus far ignored the comment at the top of the file.) -- Father Chrysostomos


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org