Skip Menu |
Report information
Id: 131730
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: pali [at] cpan.org
Cc:
AdminCc:

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



Subject: "exec PROGRAM LIST" does not work with empty list
To: perlbug [...] perl.org
Date: Mon, 10 Jul 2017 14:07:54 +0200
From: pali [...] cpan.org
Download (untitled) / with headers
text/plain 985b
When exec is called with empty list, either via explicit () or implicit, from empty array variable, then it always fail and set $! to empty string. See examples: $ perl -e 'exec {"binary"} () or die "exec failed: \"$!\"\n";' exec failed: "" $ perl -e 'my @args; exec {"binary"} @args or die "exec failed: \"$!\"\n";' exec failed: "" Here is patch which fixes it: diff --git a/doio.c b/doio.c index 6f4cd84f8c..32f6e3416f 100644 --- a/doio.c +++ b/doio.c @@ -1583,7 +1583,7 @@ 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 (sp > mark) { + if (sp >= mark) { const char **a; const char *tmps = NULL; Newx(PL_Argv, sp - mark + 1, const char*); With above patch it is working fine: perl -e 'exec {"true"} () or die "exec failed: \"$!\"\n";'; echo $? 0 perl -e 'exec {"false"} () or die "exec failed: \"$!\"\n";'; echo $? 1
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
Date: Mon, 10 Jul 2017 14:57:38 +0100
To: perl5-porters [...] perl.org
Subject: Re: [perl #131730] "exec PROGRAM LIST" does not work with empty list
Download (untitled) / with headers
text/plain 3.7k
(via RT) <perlbug-followup@perl.org> writes: Show quoted text
> When exec is called with empty list, either via explicit () or implicit, > from empty array variable, then it always fail and set $! to empty > string. See examples: > > $ perl -e 'exec {"binary"} () or die "exec failed: \"$!\"\n";' > exec failed: "" > > $ perl -e 'my @args; exec {"binary"} @args or die "exec failed: \"$!\"\n";' > exec failed: "" > > Here is patch which fixes it: > > diff --git a/doio.c b/doio.c > index 6f4cd84f8c..32f6e3416f 100644 > --- a/doio.c > +++ b/doio.c > @@ -1583,7 +1583,7 @@ 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 (sp > mark) { > + if (sp >= mark) { > const char **a; > const char *tmps = NULL; > Newx(PL_Argv, sp - mark + 1, const char*); > > With above patch it is working fine: > > perl -e 'exec {"true"} () or die "exec failed: \"$!\"\n";'; echo $? > 0 > > perl -e 'exec {"false"} () or die "exec failed: \"$!\"\n";'; echo $? > 1
This causes calling execvp() with an empty argv array, which is not strictly POSIX-conforming (it "should point to a filename"), and might break programs that expect to be able to access argv[0] without checking argc, but I agree perl should still allow it. However, with this patch "exec {''} ()" and "exec()" segfault a bit further down, trying to dereference PL_Argv[0], which is the terminating NULL pointer. exec() crashes on line 1601 while exec {''} () crashes on line 1608: if (really) tmps = SvPV_nolen_const(really); 1601: if ((!really && *PL_Argv[0] != '/') || [...] if (really && *tmps) { PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv)); } else { 1608: PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv)); } It gets to that path insted of the "really" branch, because tmps is an empty string so, *tmps is the terminating NUL byte (this behaviour of ignoring the return value of the block if it's empty is not documented, but has been there since perl 3.0). I propose explicitly returning ENOENT (which is what execvp() returns if the file name is an empty string) in both of the above failing cases, i.e. something like this: diff --git a/doio.c b/doio.c index 6f4cd84f8c..cbaf7e6a33 100644 --- a/doio.c +++ b/doio.c @@ -1583,7 +1583,7 @@ 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 (sp > mark) { + if (sp >= mark) { const char **a; const char *tmps = NULL; Newx(PL_Argv, sp - mark + 1, const char*); @@ -1598,17 +1598,19 @@ 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 && PL_Argv[0] && *PL_Argv[0] != '/') || (really && *tmps != '/')) /* will execvp use PATH? */ TAINT_ENV(); /* testing IFS here is overkill, probably */ PERL_FPU_PRE_EXEC if (really && *tmps) { PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv)); - } else { + } else if (PL_Argv[0]) { PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv)); - } + } else { + SETERRNO(ENOENT,LIB_INVARG); + } PERL_FPU_POST_EXEC - S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0]), fd, do_report); + S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0] ? PL_Argv[0] : ""), fd, do_report); } do_execfree(); #endif - ilmari -- - Twitter seems more influential [than blogs] in the 'gets reported in the mainstream press' sense at least. - Matt McLeod - That'd be because the content of a tweet is easier to condense down to a mainstream media article. - Calle Dybedahl
To: perlbug-followup [...] perl.org
Subject: Re: [perl #131730] "exec PROGRAM LIST" does not work with empty list
From: pali [...] cpan.org
Date: Thu, 13 Jul 2017 10:52:41 +0200
Download (untitled) / with headers
text/plain 4.3k
On Monday 10 July 2017 06:58:10 Dagfinn Ilmari Mannsåker via RT wrote: Show quoted text
> (via RT) <perlbug-followup@perl.org> writes: >
> > When exec is called with empty list, either via explicit () or implicit, > > from empty array variable, then it always fail and set $! to empty > > string. See examples: > > > > $ perl -e 'exec {"binary"} () or die "exec failed: \"$!\"\n";' > > exec failed: "" > > > > $ perl -e 'my @args; exec {"binary"} @args or die "exec failed: \"$!\"\n";' > > exec failed: "" > > > > Here is patch which fixes it: > > > > diff --git a/doio.c b/doio.c > > index 6f4cd84f8c..32f6e3416f 100644 > > --- a/doio.c > > +++ b/doio.c > > @@ -1583,7 +1583,7 @@ 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 (sp > mark) { > > + if (sp >= mark) { > > const char **a; > > const char *tmps = NULL; > > Newx(PL_Argv, sp - mark + 1, const char*); > > > > With above patch it is working fine: > > > > perl -e 'exec {"true"} () or die "exec failed: \"$!\"\n";'; echo $? > > 0 > > > > perl -e 'exec {"false"} () or die "exec failed: \"$!\"\n";'; echo $? > > 1
> > This causes calling execvp() with an empty argv array, which is not > strictly POSIX-conforming (it "should point to a filename"),
I cannot currently find POSIX requirement that first arg cannot be NULL. Show quoted text
> and might > break programs that expect to be able to access argv[0] without checking > argc, but I agree perl should still allow it.
But, in ISO C89, ISO C99 and also ISO C11, main's argc can be zero. In all those ISO C standards is written: * The value of argc shall be nonnegative. * argv[argc] shall be a null pointer. Show quoted text
> However, with this patch "exec {''} ()" and "exec()" segfault a bit > further down, trying to dereference PL_Argv[0], which is the terminating > NULL pointer.
Ouh, I have not caught this. Show quoted text
> exec() crashes on line 1601 while exec {''} () crashes on line 1608: > > if (really) > tmps = SvPV_nolen_const(really); > 1601: if ((!really && *PL_Argv[0] != '/') || > [...] > if (really && *tmps) { > PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv)); > } else { > 1608: PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv)); > } > > It gets to that path insted of the "really" branch, because tmps is an > empty string so, *tmps is the terminating NUL byte (this behaviour of > ignoring the return value of the block if it's empty is not documented, > but has been there since perl 3.0). > > I propose explicitly returning ENOENT (which is what execvp() returns if > the file name is an empty string) in both of the above failing cases, > i.e. something like this:
Looks good, thanks for update! Show quoted text
> diff --git a/doio.c b/doio.c > index 6f4cd84f8c..cbaf7e6a33 100644 > --- a/doio.c > +++ b/doio.c > @@ -1583,7 +1583,7 @@ 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 (sp > mark) { > + if (sp >= mark) { > const char **a; > const char *tmps = NULL; > Newx(PL_Argv, sp - mark + 1, const char*); > @@ -1598,17 +1598,19 @@ 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 && PL_Argv[0] && *PL_Argv[0] != '/') || > (really && *tmps != '/')) /* will execvp use PATH? */ > TAINT_ENV(); /* testing IFS here is overkill, probably */ > PERL_FPU_PRE_EXEC > if (really && *tmps) { > PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv)); > - } else { > + } else if (PL_Argv[0]) { > PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv)); > - } > + } else { > + SETERRNO(ENOENT,LIB_INVARG); > + } > PERL_FPU_POST_EXEC > - S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0]), fd, do_report); > + S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0] ? PL_Argv[0] : ""), fd, do_report); > } > do_execfree(); > #endif > > - ilmari > -- > - Twitter seems more influential [than blogs] in the 'gets reported in > the mainstream press' sense at least. - Matt McLeod > - That'd be because the content of a tweet is easier to condense down > to a mainstream media article. - Calle Dybedahl
To: perl5-porters [...] perl.org
Subject: Re: [perl #131730] "exec PROGRAM LIST" does not work with empty list
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
Date: Thu, 13 Jul 2017 10:35:21 +0100
pali@cpan.org writes: Show quoted text
> On Monday 10 July 2017 06:58:10 Dagfinn Ilmari Mannsåker via RT wrote:
>> This causes calling execvp() with an empty argv array, which is not >> strictly POSIX-conforming (it "should point to a filename"),
> > I cannot currently find POSIX requirement that first arg cannot be NULL.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html says: | The value in argv[0] should point to a filename string that is | associated with the process being started by one of the exec | functions. It's only a "should", not a "must", so the requirement only applies to Strictly Conforming POSIX Applications, which we can't require perl scripts to be. Show quoted text
>> and might >> break programs that expect to be able to access argv[0] without checking >> argc, but I agree perl should still allow it.
> > But, in ISO C89, ISO C99 and also ISO C11, main's argc can be zero. In > all those ISO C standards is written: > > * The value of argc shall be nonnegative. > * argv[argc] shall be a null pointer.
The RATIONALE section of the POSIX exec() man page explains the history: | Early proposals required that the value of argc passed to main() be "one | or greater". This was driven by the same requirement in drafts of the | ISO C standard. In fact, historical implementations have passed a value | of zero when no arguments are supplied to the caller of the exec | functions. This requirement was removed from the ISO C standard and | subsequently removed from this volume of POSIX.1-2008 as well. The | wording, in particular the use of the word should, requires a Strictly | Conforming POSIX Application to pass at least one argument to the exec | function, thus guaranteeing that argc be one or greater when invoked by | such an application. In fact, this is good practice, since many existing | applications reference argv[0] without first checking the value of argc. […] Show quoted text
>> I propose explicitly returning ENOENT (which is what execvp() returns if >> the file name is an empty string) in both of the above failing cases, >> i.e. something like this:
> > Looks good, thanks for update!
I'm going on holiday for a week today, but unless anyone objects (or beats me to it) I'll write some tests and push this when I get back. Show quoted text
>> diff --git a/doio.c b/doio.c >> index 6f4cd84f8c..cbaf7e6a33 100644 >> --- a/doio.c >> +++ b/doio.c >> @@ -1583,7 +1583,7 @@ 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 (sp > mark) { >> + if (sp >= mark) { >> const char **a; >> const char *tmps = NULL; >> Newx(PL_Argv, sp - mark + 1, const char*); >> @@ -1598,17 +1598,19 @@ 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 && PL_Argv[0] && *PL_Argv[0] != '/') || >> (really && *tmps != '/')) /* will execvp use PATH? */ >> TAINT_ENV(); /* testing IFS here is overkill, probably */ >> PERL_FPU_PRE_EXEC >> if (really && *tmps) { >> PerlProc_execvp(tmps,EXEC_ARGV_CAST(PL_Argv)); >> - } else { >> + } else if (PL_Argv[0]) { >> PerlProc_execvp(PL_Argv[0],EXEC_ARGV_CAST(PL_Argv)); >> - } >> + } else { >> + SETERRNO(ENOENT,LIB_INVARG); >> + } >> PERL_FPU_POST_EXEC >> - S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0]), fd, do_report); >> + S_exec_failed(aTHX_ (really ? tmps : PL_Argv[0] ? PL_Argv[0] : ""), fd, do_report); >> } >> do_execfree(); >> #endif >> >> - ilmari >> -- >> - Twitter seems more influential [than blogs] in the 'gets reported in >> the mainstream press' sense at least. - Matt McLeod >> - That'd be because the content of a tweet is easier to condense down >> to a mainstream media article. - Calle Dybedahl
>
-- "The surreality of the universe tends towards a maximum" -- Skud's Law "Never formulate a law or axiom that you're not prepared to live with the consequences of." -- Skud's Meta-Law
Subject: Re: [perl #131730] "exec PROGRAM LIST" does not work with empty list
From: pali [...] cpan.org
To: perlbug-followup [...] perl.org
Date: Wed, 9 Aug 2017 14:54:48 +0200
Download (untitled) / with headers
text/plain 266b
On Thursday 13 July 2017 10:14:41 Dagfinn Ilmari Mannsåker via RT wrote: Show quoted text
> I'm going on holiday for a week today, but unless anyone objects (or > beats me to it) I'll write some tests and push this when I get back.
Hi! If you are back, can you process this patch?
From: pali [...] cpan.org
Subject: Re: [perl #131730] "exec PROGRAM LIST" does not work with empty list
Date: Mon, 2 Oct 2017 19:43:51 +0200
To: perlbug-followup [...] perl.org
Hi! I would like to remind patch in this ticket.
To: perl5-porters [...] perl.org
Date: Wed, 18 Oct 2017 18:58:44 +0100
Subject: Re: [perl #131730] "exec PROGRAM LIST" does not work with empty list
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
Download (untitled) / with headers
text/plain 628b
pali@cpan.org writes: Show quoted text
> On Thursday 13 July 2017 10:14:41 Dagfinn Ilmari Mannsåker via RT wrote:
>> I'm going on holiday for a week today, but unless anyone objects (or >> beats me to it) I'll write some tests and push this when I get back.
> > Hi! If you are back, can you process this patch?
Sorry for the delay. I've now merged the patch, and it'll be in the upcoming 5.27.5 development release. - ilmari -- "The surreality of the universe tends towards a maximum" -- Skud's Law "Never formulate a law or axiom that you're not prepared to live with the consequences of." -- Skud's Meta-Law
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 149b
Since we haven't observed any test failures since Ilmari committed the patch, I'm marking this ticket Resolved. -- James E Keenan (jkeenan@cpan.org)


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