Skip Menu |
Report information
Id: 131746
Status: pending release
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)

Attachments
0001-perl-131746-avoid-undefined-behaviour-in-Copy-etc.patch



Date: Wed, 12 Jul 2017 13:51:03 -0500
From: Brian Carpenter <brian.carpenter [...] gmail.com>
Subject: runtime error: null pointer passed as argument 1, which is declared to never be null (pp_hot.c:4147:6)
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 1.9k
While compiling 1e629c2 for the purposes of fuzzing, I encountered some "Undefined Behavior" which should probably be fixed.

Command line:
./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-clang-fast -Doptimize=-O2\ -g -Accflags='-fsanitize=address,undefined -fno-omit-frame-pointer' -Aldflags='-fsanitize=address,undefined -fno-omit-frame-pointer' && AFL_TRACE_PC=1 AFL_USE_ASAN=1 make

Error:
afl-clang-fast [tpcg] 2.46b 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'
pp_hot.c:4147:6: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:47:28: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior pp_hot.c:4147:6

I tried to get a stack trace, but this is all UBSan would give up:
UBSAN_OPTIONS=symbolize=1:halt_on_error=1:abort_on_error=1:print_stacktrace=1 AFL_TRACE_PC=1 AFL_USE_ASAN=1 make
./miniperl -Ilib make_ext.pl cpan/Archive-Tar/pm_to_blib  MAKE="make" LIBPERL_A=libperl.a
pp_hot.c:4147:6: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:47:28: note: nonnull attribute specified here
    #0 0xeff26a  (/root/perl/miniperl+0xeff26a)
    #1 0x69d223  (/root/perl/miniperl+0x69d223)
    #2 0x677d0c  (/root/perl/miniperl+0x677d0c)
    #3 0x5ce100  (/root/perl/miniperl+0x5ce100)
    #4 0x559854  (/root/perl/miniperl+0x559854)
    #5 0x9642ff  (/root/perl/miniperl+0x9642ff)
    #6 0x68d51f  (/root/perl/miniperl+0x68d51f)
    #7 0x67d050  (/root/perl/miniperl+0x67d050)
    #8 0x17dd231  (/root/perl/miniperl+0x17dd231)
    #9 0x7fbe01104b44  (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
    #10 0x41ef5b  (/root/perl/miniperl+0x41ef5b)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior pp_hot.c:4147:6 in
Aborted
makefile:586: recipe for target 'cpan/Archive-Tar/pm_to_blib' failed
make: *** [cpan/Archive-Tar/pm_to_blib] Error 134
From: Tony Cook <tony [...] develop-help.com>
Date: Thu, 13 Jul 2017 15:01:25 +1000
Subject: Re: [perl #131746] runtime error: null pointer passed as argument 1, which is declared to never be null (pp_hot.c:4147:6)
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 3.4k
On Wed, Jul 12, 2017 at 11:51:52AM -0700, Brian Carpenter wrote: Show quoted text
> # New Ticket Created by Brian Carpenter > # Please include the string: [perl #131746] > # in the subject line of all future correspondence about this issue. > # <URL: https://rt.perl.org/Ticket/Display.html?id=131746 > > > > While compiling 1e629c2 for the purposes of fuzzing, I encountered some > "Undefined Behavior" which should probably be fixed. > > Command line: > ./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-clang-fast -Doptimize=-O2\ > -g -Accflags='-fsanitize=address,undefined -fno-omit-frame-pointer' > -Aldflags='-fsanitize=address,undefined -fno-omit-frame-pointer' && > AFL_TRACE_PC=1 AFL_USE_ASAN=1 make > > Error: > afl-clang-fast [tpcg] 2.46b 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'
> pp_hot.c:4147:6: runtime error: null pointer passed as argument 1, which is > declared to never be null > /usr/include/string.h:47:28: note: nonnull attribute specified here > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior pp_hot.c:4147:6 > > I tried to get a stack trace, but this is all UBSan would give up: > UBSAN_OPTIONS=symbolize=1:halt_on_error=1:abort_on_error=1:print_stacktrace=1 > AFL_TRACE_PC=1 AFL_USE_ASAN=1 make > ./miniperl -Ilib make_ext.pl cpan/Archive-Tar/pm_to_blib MAKE="make" > LIBPERL_A=libperl.a > pp_hot.c:4147:6: runtime error: null pointer passed as argument 1, which is > declared to never be null > /usr/include/string.h:47:28: note: nonnull attribute specified here > #0 0xeff26a (/root/perl/miniperl+0xeff26a) > #1 0x69d223 (/root/perl/miniperl+0x69d223) > #2 0x677d0c (/root/perl/miniperl+0x677d0c) > #3 0x5ce100 (/root/perl/miniperl+0x5ce100) > #4 0x559854 (/root/perl/miniperl+0x559854) > #5 0x9642ff (/root/perl/miniperl+0x9642ff) > #6 0x68d51f (/root/perl/miniperl+0x68d51f) > #7 0x67d050 (/root/perl/miniperl+0x67d050) > #8 0x17dd231 (/root/perl/miniperl+0x17dd231) > #9 0x7fbe01104b44 (/lib/x86_64-linux-gnu/libc.so.6+0x21b44) > #10 0x41ef5b (/root/perl/miniperl+0x41ef5b) > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior pp_hot.c:4147:6 in > Aborted > makefile:586: recipe for target 'cpan/Archive-Tar/pm_to_blib' failed > make: *** [cpan/Archive-Tar/pm_to_blib] Error 134
I haven't been able to reproduce this. To get more information you might try it under a debugger, so something like: ASAN_OPTIONS=abort_on_error=1 gdb --args ./miniperl -Ilib make_ext.pl cpan/Archive-Tar/pm_to_blib MAKE="make" LIBPERL_A=libperl.a Given the line number, this code: Copy(MARK+1,AvARRAY(av),items,SV*); is the problem, but I don't see how MARK can be NULL (and MARK+1 won't be NULL). So it's likely AvARRAY(av) is NULL, and in this case items is probably zero. Copy() is essentially a wrapper around memcpy(), and from this message: Show quoted text
> /usr/include/string.h:47:28: note: nonnull attribute specified here
it looks like memcpy() has NULL restrictions on its pointer parameters (I found none in my local string.h), which seemed kind of questionable, but both: https://stackoverflow.com/questions/5243012/is-it-guaranteed-to-be-safe-to-perform-memcpy0-0-0 and https://www.imperialviolet.org/2016/06/26/nonnull.html make a good argument that it isn't. Can you reproduce the problem if you add a conditional to that line, eg: if (items) Copy(MARK+1,AvARRAY(av),items,SV*); Tony
Subject: Re: [perl #131746] runtime error: null pointer passed as argument 1, which is declared to never be null (pp_hot.c:4147:6)
To: perlbug-followup [...] perl.org
Date: Thu, 13 Jul 2017 00:54:33 -0500
From: Brian Carpenter <brian.carpenter [...] gmail.com>
Download (untitled) / with headers
text/plain 2.1k
llvm-symbolizer wasn't in my $PATH on this particular machine. Once I solved that oversight:

afl-clang-fast [tpcg] 2.46b by <lszekeres@google.com>
afl-clang-fast -fstack-protector-strong -L/usr/local/lib -fsanitize=address,undefined -fno-omit-frame-pointer -fsanitize=address,undefined -fno-omit-frame-pointer -fsanitize=address,undefined -fno-omit-frame-pointer -fsanitize=address,undefined -fno-omit-frame-pointer -fsanitize=address,undefined -fno-omit-frame-pointer -fsanitize=address,undefined -fno-omit-frame-pointer -o miniperl \
    opmini.o perlmini.o  gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o reentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp_ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o globals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o caretx.o dquote.o time64.o  miniperlmain.o  -lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
afl-clang-fast [tpcg] 2.46b 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'
pp_hot.c:4147:6: runtime error: null pointer passed as argument 1, which is declared to never be null
/usr/include/string.h:47:28: note: nonnull attribute specified here
    #0 0xeff26a in Perl_pp_entersub /root/perl/pp_hot.c:4147:6
    #1 0x69d223 in Perl_call_sv /root/perl/perl.c:2872:6
    #2 0x677d0c in Perl_call_list /root/perl/perl.c:5043:6
    #3 0x5ce100 in S_process_special_blocks /root/perl/op.c:9058:6
    #4 0x559854 in Perl_newATTRSUB_x /root/perl/op.c:8987:21
    #5 0x9642ff in Perl_yyparse /root/perl/perly.y:302:12
    #6 0x68d51f in S_parse_body /root/perl/perl.c:2401:9
    #7 0x67d050 in perl_parse /root/perl/perl.c:1719:2
    #8 0x17dd371 in main /root/perl/miniperlmain.c:127:18
    #9 0x7fde4561cb44 in __libc_start_main /build/glibc-6V9RKT/glibc-2.19/csu/libc-start.c:287
    #10 0x41ef5b in _start (/root/perl/miniperl+0x41ef5b)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior pp_hot.c:4147:6 in
Aborted
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 276b
On Wed, 12 Jul 2017 22:56:09 -0700, brian.carpenter@gmail.com wrote: Show quoted text
> llvm-symbolizer wasn't in my $PATH on this particular machine. Once I > solved that oversight:
Thanks, I'm pretty sure the fix in my previous response will solve it, could you please try it? Thanks, Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 348b
Sorry for the delay, your change does indeed solve the problem I was having and I was able to compile perl v5.27.2-135-g7aaa36b196* without this particular UBSan message. On Thu, 13 Jul 2017 17:49:55 -0700, tonyc wrote: Show quoted text
> > Thanks, I'm pretty sure the fix in my previous response will solve it, > could you please try it? > > Thanks, > Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 331b
On Sun, 13 Aug 2017 14:58:16 -0700, brian.carpenter@gmail.com wrote: Show quoted text
> Sorry for the delay, your change does indeed solve the problem I was > having and I was able to compile perl v5.27.2-135-g7aaa36b196* without > this particular UBSan message.
The attached should fix both this and #131892. Please let me know if it does. Tony
Subject: 0001-perl-131746-avoid-undefined-behaviour-in-Copy-etc.patch
From 4f049cb3136c9916ededf0416d6439a4838ae7c8 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Mon, 14 Aug 2017 11:52:39 +1000 Subject: (perl #131746) avoid undefined behaviour in Copy() etc These functions depend on C library functions which have undefined behaviour when passed NULL pointers, even when passed a zero 'n' value. Some compilers use this information, ie. assume the pointers are non-NULL when optimizing any following code, so we do need to prevent such unguarded calls. My initial thought was to add conditionals to each macro to skip the call to the library function when n is zero, but this adds a cost to every use of these macros, even when the n value is always true. So instead I added asserts() which will give us a much more visible indicator of such broken code and revealed the pp_caller and Glob.xs issues also patched here. --- ext/File-Glob/Glob.pm | 2 +- ext/File-Glob/Glob.xs | 2 +- handy.h | 14 +++++++------- pp_ctl.c | 3 ++- pp_hot.c | 3 ++- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/ext/File-Glob/Glob.pm b/ext/File-Glob/Glob.pm index 4f74023..6614e82 100644 --- a/ext/File-Glob/Glob.pm +++ b/ext/File-Glob/Glob.pm @@ -37,7 +37,7 @@ pop @{$EXPORT_TAGS{bsd_glob}}; # no "glob" @EXPORT_OK = (@{$EXPORT_TAGS{'glob'}}, 'csh_glob'); -$VERSION = '1.29'; +$VERSION = '1.30'; sub import { require Exporter; diff --git a/ext/File-Glob/Glob.xs b/ext/File-Glob/Glob.xs index e0a3681..9779d54 100644 --- a/ext/File-Glob/Glob.xs +++ b/ext/File-Glob/Glob.xs @@ -121,7 +121,7 @@ iterate(pTHX_ bool(*globber)(pTHX_ AV *entries, const char *pat, STRLEN len, boo /* chuck it all out, quick or slow */ if (gimme == G_ARRAY) { - if (!on_stack) { + if (!on_stack && AvFILLp(entries) + 1) { EXTEND(SP, AvFILLp(entries)+1); Copy(AvARRAY(entries), SP+1, AvFILLp(entries)+1, SV *); SP += AvFILLp(entries)+1; diff --git a/handy.h b/handy.h index c3848bf..7ef7e25 100644 --- a/handy.h +++ b/handy.h @@ -2409,17 +2409,17 @@ void Perl_mem_log_del_sv(const SV *sv, const char *filename, const int linenumbe #define Safefree(d) safefree(MEM_LOG_FREE((Malloc_t)(d))) #endif -#define Move(s,d,n,t) (MEM_WRAP_CHECK_(n,t) (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) -#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) -#define Zero(d,n,t) (MEM_WRAP_CHECK_(n,t) (void)memzero((char*)(d), (n) * sizeof(t))) +#define Move(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) +#define Copy(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), (void)memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) +#define Zero(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), (void)memzero((char*)(d), (n) * sizeof(t))) -#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) -#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) +#define MoveD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memmove((char*)(d),(const char*)(s), (n) * sizeof(t))) +#define CopyD(s,d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), assert(s), memcpy((char*)(d),(const char*)(s), (n) * sizeof(t))) #ifdef HAS_MEMSET -#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) memzero((char*)(d), (n) * sizeof(t))) +#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t))) #else /* Using bzero(), which returns void. */ -#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) memzero((char*)(d), (n) * sizeof(t)),d) +#define ZeroD(d,n,t) (MEM_WRAP_CHECK_(n,t) assert(d), memzero((char*)(d), (n) * sizeof(t)),d) #endif #define PoisonWith(d,n,t,b) (MEM_WRAP_CHECK_(n,t) (void)memset((char*)(d), (U8)(b), (n) * sizeof(t))) diff --git a/pp_ctl.c b/pp_ctl.c index f91bb4d..3e8c7b4 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -1997,7 +1997,8 @@ PP(pp_caller) if (AvMAX(PL_dbargs) < AvFILLp(ary) + off) av_extend(PL_dbargs, AvFILLp(ary) + off); - Copy(AvALLOC(ary), AvARRAY(PL_dbargs), AvFILLp(ary) + 1 + off, SV*); + if (AvFILLp(ary) + 1 + off) + Copy(AvALLOC(ary), AvARRAY(PL_dbargs), AvFILLp(ary) + 1 + off, SV*); AvFILLp(PL_dbargs) = AvFILLp(ary) + off; } mPUSHi(CopHINTS_get(cx->blk_oldcop)); diff --git a/pp_hot.c b/pp_hot.c index 528817f..95bdf54 100644 --- a/pp_hot.c +++ b/pp_hot.c @@ -4333,7 +4333,8 @@ PP(pp_entersub) AvARRAY(av) = ary; } - Copy(MARK+1,AvARRAY(av),items,SV*); + if (items) + Copy(MARK+1,AvARRAY(av),items,SV*); AvFILLp(av) = items - 1; } if (UNLIKELY((cx->blk_u16 & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO && -- 2.1.4
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 154b
LGTM On Sun, 13 Aug 2017 18:54:27 -0700, tonyc wrote: Show quoted text
> > The attached should fix both this and #131892. > > Please let me know if it does. > > Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 151b
On Sun, 13 Aug 2017 19:10:27 -0700, brian.carpenter@gmail.com wrote: Show quoted text
> LGTM
Thanks, I've applied it as f14cf3632059d421de83cf901c7e849adc1fcd03. Tony
Date: Mon, 4 Sep 2017 11:10:52 +0100
CC: perl5-porters [...] perl.org
To: Tony Cook via RT <perlbug-followup [...] perl.org>
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #131746] runtime error: null pointer passed as argument 1, which is declared to never be null (pp_hot.c:4147:6)
Download (untitled) / with headers
text/plain 1003b
On Sun, Sep 03, 2017 at 10:08:55PM -0700, Tony Cook via RT wrote: Show quoted text
> On Sun, 13 Aug 2017 19:10:27 -0700, brian.carpenter@gmail.com wrote:
> > LGTM
> > Thanks, I've applied it as f14cf3632059d421de83cf901c7e849adc1fcd03.
The assert shows up a problem in threads.xs which was breaking threaded builds; I've just fixed that with commit 97fcda75b598695644a4ad496e090941f5b7dcbc Author: David Mitchell <davem@iabyn.com> AuthorDate: Mon Sep 4 09:54:58 2017 +0100 Commit: David Mitchell <davem@iabyn.com> CommitDate: Mon Sep 4 11:09:06 2017 +0100 threads.xs: don't Copy() null pointer If a thread is created with no parameters, e.g. use threads; threads->new(sub {})->join; Then it tries to Copy() zero parameters to AvARRAY(params), which is null. Since v5.27.3-31-gf14cf36, this triggers an assert failure, so threaded builds have been badly broken. -- In economics, the exam questions are the same every year. They just change the answers.


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