Skip Menu |
Report information
Id: 129888
Status: resolved
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)



Date: Sat, 15 Oct 2016 17:22:13 -0500
To: perlbug [...] perl.org
Subject: null ptr deref, segfault in Perl_do_aexec5 (doio.c:1595)
From: "Brian 'geeknik' Carpenter" <brian.carpenter [...] gmail.com>
Download (untitled) / with headers
text/plain 4.6k
Triggered with AFL+ASAN in Perl v5.25.6 (v5.25.5-104-gaff2be5).

od -tx1 test274
0000000 7b 24 30 3d 24 53 49 47 7b 5f 5f 57 41 52 4e 5f
0000020 5f 7d 3d 73 75 62 7b 65 78 65 63 7d 3b 24 5e 57
0000040 3d 32 2c 65 78 65 63 24 70 00 30 7d
0000054

Number found where operator expected at test274 line 1, near "$p0"
        (Missing operator before 0?)
ASAN:SIGSEGV
=================================================================
==6592==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000b30ea7 bp 0x7fffff249420 sp 0x7fffff249280 T0)
    #0 0xb30ea6 in Perl_do_aexec5 /root/perl/doio.c:1595:13
    #1 0xaf9bbc in Perl_pp_exec /root/perl/pp_sys.c:4515:15
    #2 0x7f44b3 in Perl_runops_debug /root/perl/dump.c:2246:23
    #3 0x5a12b6 in S_run_body /root/perl/perl.c:2526:2
    #4 0x5a12b6 in perl_run /root/perl/perl.c:2449
    #5 0x4de60d in main /root/perl/perlmain.c:123:9
    #6 0x7f7211793b44 in __libc_start_main /build/glibc-daoqzt/glibc-2.19/csu/libc-start.c:287
    #7 0x4de27c in _start (/root/perl/perl+0x4de27c)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /root/perl/doio.c:1595 Perl_do_aexec5
==6592==ABORTING

Number found where operator expected at test274 line 1, near "$p0"
        (Missing operator before 0?)
==6567== Invalid read of size 8
==6567==    at 0x5A7B17: Perl_do_aexec5 (doio.c:1595)
==6567==    by 0x598CD4: Perl_pp_exec (pp_sys.c:4515)
==6567==    by 0x4D71A1: Perl_runops_debug (dump.c:2246)
==6567==    by 0x453156: S_run_body (perl.c:2526)
==6567==    by 0x453156: perl_run (perl.c:2449)
==6567==    by 0x421984: main (perlmain.c:123)
==6567==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==6567==
==6567==
==6567== Process terminating with default action of signal 11 (SIGSEGV)
==6567==  Access not within mapped region at address 0x0
==6567==    at 0x5A7B17: Perl_do_aexec5 (doio.c:1595)
==6567==    by 0x598CD4: Perl_pp_exec (pp_sys.c:4515)
==6567==    by 0x4D71A1: Perl_runops_debug (dump.c:2246)
==6567==    by 0x453156: S_run_body (perl.c:2526)
==6567==    by 0x453156: perl_run (perl.c:2449)
==6567==    by 0x421984: main (perlmain.c:123)
==6567==  If you believe this happened as a result of a stack
==6567==  overflow in your program's main thread (unlikely but
==6567==  possible), you can try to increase the size of the
==6567==  main thread stack using the --main-stacksize= flag.
==6567==  The main thread stack size used in this run was 8388608.
Segmentation fault

Number found where operator expected at test274 line 1, near "$p0"
        (Missing operator before 0?)

Program received signal SIGSEGV, Segmentation fault.
0x00000000005a7b17 in Perl_do_aexec5 (really=0x939998, mark=0x919b70, mark@entry=0x919b60, sp=sp@entry=0x919b68, fd=fd@entry=0, do_report=do_report@entry=0) at doio.c:1595
1595                PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv));
(gdb) bt
#0  0x00000000005a7b17 in Perl_do_aexec5 (really=0x939998, mark=0x919b70, mark@entry=0x919b60, sp=sp@entry=0x919b68, fd=fd@entry=0, do_report=do_report@entry=0) at doio.c:1595
#1  0x0000000000598cd5 in Perl_pp_exec () at pp_sys.c:4515
#2  0x00000000004d71a2 in Perl_runops_debug () at dump.c:2246
#3  0x0000000000453157 in S_run_body (oldscope=1) at perl.c:2526
#4  perl_run (my_perl=<optimized out>) at perl.c:2449
#5  0x0000000000421985 in main (argc=2, argv=0x7fffffffe6b8, env=0x7fffffffe6d0) at perlmain.c:123
(gdb) list
1590                TAINT_ENV();                /* testing IFS here is overkill, probably */
1591            PERL_FPU_PRE_EXEC
1592            if (really && *tmps) {
1593                PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv));
1594            } else {
1595                PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv));
1596            }
1597            PERL_FPU_POST_EXEC
1598            S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0]), fd, do_report);
1599        }
(gdb) i r
rax            0x0      0
rbx            0x6193c4 6394820
rcx            0x0      0
rdx            0x7ffff6cf20e0   140737334157536
rsi            0x7fffffffe140   140737488347456
rdi            0x8      8
rbp            0x7fffffffe360   0x7fffffffe360
rsp            0x7fffffffe350   0x7fffffffe350
r8             0x7fffffffe360   140737488348000
r9             0x7fffffffe1e0   140737488347616
r10            0x8      8
r11            0x206    518
r12            0x932d10 9645328
r13            0x939998 9673112
r14            0x932d20 9645344
r15            0x919b70 9542512
rip            0x5a7b17 0x5a7b17 <Perl_do_aexec5+359>
eflags         0x10246  [ PF ZF IF RF ]
cs             0x33     51
ss             0x2b     43
ds             0x0      0
es             0x0      0
fs             0x0      0
gs             0x0      0
Download test274.gz
application/x-gzip 69b

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.9k
In short: % ./miniperl -we '$SIG{__WARN__} = sub { exec }; exec $p 0;' Name "main::p" used only once: possible typo at -e line 1. Segmentation fault (core dumped) % Fetching the exec arguments in do_aexec5() can trigger arbitrary perl code, which can cause the PL_Argv array we're building up to get removed before we're done. I'm not sure what's the best approach here; here's a couple of possibilities: # disallow reentrant calls: UNLIKELY(breaks some working code) --- a/doio.c +++ b/doio.c @@ -1574,6 +1574,8 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp, #if defined(__SYMBIAN32__) || defined(__LIBCATAMOUNT__) Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system"); #else + if (PL_Argv) + Perl_croak(aTHX_ "panic: do_aexec5() called reentrantly"); if (sp > mark) { const char **a; const char *tmps = NULL; # set PL_Argv as late as possible: UNLIKELY(leaks some memory) --- a/doio.c +++ b/doio.c @@ -1575,10 +1575,11 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp, Perl_croak(aTHX_ "exec? I'm not *that* kind of operating system"); #else if (sp > mark) { + const char **argv; const char **a; const char *tmps = NULL; - Newx(PL_Argv, sp - mark + 1, const char*); - a = PL_Argv; + Newx(argv, sp - mark + 1, const char*); + a = argv; while (++mark <= sp) { if (*mark) @@ -1589,10 +1590,11 @@ Perl_do_aexec5(pTHX_ SV *really, SV **mark, SV **sp, *a = NULL; if (really) tmps = SvPV_nolen_const(really); - if ((!really && *PL_Argv[0] != '/') || + if ((!really && *argv[0] != '/') || (really && *tmps != '/')) /* will execvp use PATH? */ TAINT_ENV(); /* testing IFS here is overkill, probably */ PERL_FPU_PRE_EXEC + PL_Argv = argv; if (really && *tmps) { PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv)); } else { Hugo
Date: Wed, 6 Dec 2017 21:37:50 +0000
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #129888] null ptr deref, segfault in Perl_do_aexec5 (doio.c:1595)
Download (untitled) / with headers
text/plain 581b
The globals that are causing this problem don't need to be global at all. They're only managing the freeing of that memory, and I reckon we can do better by using the scope stack to arrange freeing. I've implemented this on the branch zefram/smoke-me/exec_args. It fixes the original test case on Unix. However, there's a bunch of platform-specific code in this area, so this needs broader testing. It also needs broad testing to make sure the scope stack arrangement actually achieves correct memory management, for example around vfork and Windows's simulated fork. -zefram
Subject: Re: [perl #129888] null ptr deref, segfault in Perl_do_aexec5(doio.c:1595)
From: James E Keenan <jkeenan [...] pobox.com>
Date: Wed, 6 Dec 2017 21:40:15 -0500
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 759b
On 12/06/2017 04:37 PM, Zefram wrote: Show quoted text
> The globals that are causing this problem don't need to be global at all. > They're only managing the freeing of that memory, and I reckon we can do > better by using the scope stack to arrange freeing. I've implemented this > on the branch zefram/smoke-me/exec_args.
For whatever benefit there might be of creating a branch for smoke-testing, you probably should rename it: smoke-me/zefram/exec_args That way (a) in the rare case of someone (i.e., TonyC) who has set up automated smoke-testing set up for non-blead branches, it gets recognized and run; and (b) its results get tabulated at perl.develop-help.com (e.g., http://perl.develop-help.com/?b=smoke-me%2Fkhw-locale). Thank you very much. Jim Keenan
Date: Thu, 7 Dec 2017 19:52:53 +0000
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #129888] null ptr deref, segfault in Perl_do_aexec5(doio.c:1595)
Download (untitled) / with headers
text/plain 428b
James E Keenan wrote: Show quoted text
>For whatever benefit there might be of creating a branch for smoke-testing, >you probably should rename it:
Really? I had got the impression that the "smoke-me" segment could be anywhere in the name. I see that perlgit(1) doesn't actually say what the rules are. Could someone authoritative please give us a regexp defining what constitutes a smoke-me branch name? And put it in perlgit(1). -zefram
From: James E Keenan <jkeenan [...] pobox.com>
Subject: Re: [perl #129888] null ptr deref, segfault inPerl_do_aexec5(doio.c:1595)
Date: Thu, 7 Dec 2017 22:44:18 -0500
To: perl5-porters [...] perl.org
On 12/07/2017 02:52 PM, Zefram wrote: Show quoted text
> James E Keenan wrote:
>> For whatever benefit there might be of creating a branch for smoke-testing, >> you probably should rename it:
> > Really? I had got the impression that the "smoke-me" segment could be > anywhere in the name. I see that perlgit(1) doesn't actually say what the > rules are. Could someone authoritative please give us a regexp defining > what constitutes a smoke-me branch name? And put it in perlgit(1). >
Zefram: I've had a commit bit since 2012. How many more years do I have to serve before you consider me "authoritative"? For what it's worth, all three of the examples of a smoke-me branch presented in perlgit follow the "smoke-me/<committer>/<branch_name>" convention. In addition, if you go to http://perl.develop-help.com/, click on the drop-down that says 'blead', then click on "I want an older branch" you will see that all the branches with Tony tracks on that site begin with 'blead', 'maint' or 'smoke-me'. The <committer> part appears to be optional, but 'smoke-me' always appears first. Thank you very much. Jim Keenan
To: perl5-porters [...] perl.org
Date: Fri, 8 Dec 2017 05:31:43 +0000
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #129888] null ptr deref, segfault inPerl_do_aexec5(doio.c:1595)
Download (untitled) / with headers
text/plain 397b
James E Keenan wrote: Show quoted text
>I've had a commit bit since 2012. How many more years do I have to serve >before you consider me "authoritative"?
By "authoritative" I meant someone who maintains the smoke-me automation, and so can look at the code and tell us first-hand what the said automation actually does. It's about the role one plays, not about expertise, seniority, or other qualities. -zefram
To: Zefram <zefram [...] fysh.org>
CC: perl5-porters [...] perl.org
Date: Fri, 8 Dec 2017 17:53:58 -0600
Subject: Re: [perl #129888] null ptr deref, segfault inPerl_do_aexec5(doio.c:1595)
From: Carlos Guevara <carlos [...] carlosguevara.com>
Download (untitled) / with headers
text/plain 615b
On Thu, Dec 7, 2017 at 11:31 PM, Zefram <zefram@fysh.org> wrote:
Show quoted text
James E Keenan wrote:
>I've had a commit bit since 2012.  How many more years do I have to serve
>before you consider me "authoritative"?

By "authoritative" I meant someone who maintains the smoke-me automation,
and so can look at the code and tell us first-hand what the said
automation actually does.  It's about the role one plays, not about
expertise, seniority, or other qualities.

-zefram

I use this command on my systems:
git for-each-ref --count=3 --sort=-committerdate refs/heads/smoke-me --format='%(refname:short)'

From: Sawyer X <xsawyerx [...] gmail.com>
Date: Mon, 11 Dec 2017 14:59:57 +0200
To: James E Keenan <jkeenan [...] pobox.com>, perl5-porters [...] perl.org
Subject: Re: [perl #129888] null ptr deref, segfault inPerl_do_aexec5(doio.c:1595)
Download (untitled) / with headers
text/plain 1.2k
On 12/08/2017 05:44 AM, James E Keenan wrote: Show quoted text
> On 12/07/2017 02:52 PM, Zefram wrote:
>> James E Keenan wrote:
>>> For whatever benefit there might be of creating a branch for >>> smoke-testing, >>> you probably should rename it:
>> >> Really?  I had got the impression that the "smoke-me" segment could be >> anywhere in the name.  I see that perlgit(1) doesn't actually say >> what the >> rules are.  Could someone authoritative please give us a regexp defining >> what constitutes a smoke-me branch name?  And put it in perlgit(1). >>
> > Zefram: > > I've had a commit bit since 2012.  How many more years do I have to > serve before you consider me "authoritative"?
I think the intention here is for "definitive," as in "What does the darn regexp looks like, so we can know for sure." Funny story: I asked my brother for his car's code. His partner gave me a different code. He jumped up and said "No, that's the wrong code. You swapped two digits!" She replied with "This is the number I use every day." And it so happens that both of the numbers indeed work.[1] The regexp could be /^smoke-me/ or /smoke-me/. Either case, it would be good to know which it is. Still, I support your concern for having uniformity, regardless of where you put the "smoke-me".
To: Sawyer X <xsawyerx [...] gmail.com>
CC: James E Keenan <jkeenan [...] pobox.com>, "Perl5 Porters (E-mail)" <perl5-porters [...] perl.org>
Date: Mon, 11 Dec 2017 12:29:54 -0600
Subject: Re: [perl #129888] null ptr deref, segfault inPerl_do_aexec5(doio.c:1595)
From: "Craig A. Berry" <craig.a.berry [...] gmail.com>
Download (untitled) / with headers
text/plain 1.7k
On Mon, Dec 11, 2017 at 6:59 AM, Sawyer X <xsawyerx@gmail.com> wrote: Show quoted text
> > On 12/08/2017 05:44 AM, James E Keenan wrote:
>> On 12/07/2017 02:52 PM, Zefram wrote:
>>> I had got the impression that the "smoke-me" segment could be >>> anywhere in the name. I see that perlgit(1) doesn't actually say >>> what the >>> rules are. Could someone authoritative please give us a regexp defining >>> what constitutes a smoke-me branch name? And put it in perlgit(1).
Show quoted text
> The regexp could be /^smoke-me/ or /smoke-me/. Either case, it would be > good to know which it is. Still, I support your concern for having > uniformity, regardless of where you put the "smoke-me".
As far as I know the automatic selection of what branch or branches to smoke is external to Test::Smoke and the use of "smoke-me" in the name is just a convention to be used in whatever fashion the implementor of a smoke set-up sees fit. It's not even necessarily implemented exactly the same way in every smoke-me set-up (of which there have never been more than a couple, I think). And of course there are a number of smokers that only do blead or only blead and maint and don't do the smoke-me branches. I would be happy to discover that there is more consistency and ready-to-use code for automatic branch selection than I am aware of, but this is my impression of where we are now. That said, I've always assumed the convention was that the branch should start with "smoke-me" and the one example in perlgit does that. Eyeballing the lengthy list of existing branches at <https://perl5.git.perl.org/perl.git/heads> does not turn up any examples prior to Zefram's that puts the "smoke-me" anywhere but at the beginning. In the absence of better information, I'd say we already have a convention and this is it: "smoke-me" at the beginning.
Date: Mon, 11 Dec 2017 21:27:08 +0000
To: perl5-porters [...] perl.org
Subject: Re: [perl #129888] null ptr deref, segfault inPerl_do_aexec5(doio.c:1595)
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 258b
Carlos Guevara wrote: Show quoted text
>I use this command on my systems: >git for-each-ref --count=3 --sort=-committerdate refs/heads/smoke-me
Thanks, that makes it clear. I've renamed my branch to "smoke-me/zefram/exec_args", and pushed a patch for perlgit.pod. -zefram
From: Sawyer X <xsawyerx [...] gmail.com>
CC: James E Keenan <jkeenan [...] pobox.com>, "Perl5 Porters (E-mail)" <perl5-porters [...] perl.org>
To: "Craig A. Berry" <craig.a.berry [...] gmail.com>
Date: Tue, 12 Dec 2017 11:15:38 +0200
Subject: Re: [perl #129888] null ptr deref, segfault inPerl_do_aexec5(doio.c:1595)
Download (untitled) / with headers
text/plain 1.1k
On 12/11/2017 08:29 PM, Craig A. Berry wrote: Show quoted text
> On Mon, Dec 11, 2017 at 6:59 AM, Sawyer X <xsawyerx@gmail.com> wrote:
>> On 12/08/2017 05:44 AM, James E Keenan wrote:
>>> On 12/07/2017 02:52 PM, Zefram wrote:
>>>> I had got the impression that the "smoke-me" segment could be >>>> anywhere in the name. I see that perlgit(1) doesn't actually say >>>> what the >>>> rules are. Could someone authoritative please give us a regexp defining >>>> what constitutes a smoke-me branch name? And put it in perlgit(1).
>> The regexp could be /^smoke-me/ or /smoke-me/. Either case, it would be >> good to know which it is. Still, I support your concern for having >> uniformity, regardless of where you put the "smoke-me".
> [...] > > That said, I've always assumed the convention was that the branch > should start with "smoke-me" and the one example in perlgit does that. > Eyeballing the lengthy list of existing branches at > <https://perl5.git.perl.org/perl.git/heads> does not turn up any > examples prior to Zefram's that puts the "smoke-me" anywhere but at > the beginning. In the absence of better information, I'd say we > already have a convention and this is it: "smoke-me" at the beginning.
Agreed.
To: perl5-porters [...] perl.org
Date: Thu, 14 Dec 2017 22:54:09 +0000
Subject: Re: [perl #129888] null ptr deref, segfault in Perl_do_aexec5 (doio.c:1595)
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 195b
smoke-me testing didn't cover as many platforms as I'd hoped, but at least didn't show any relevant failures. Patch applied to blead as commit 282fc0b3cc2439f69587d980b62bef7f5d5bdfef. -zefram


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