execlp/execvp/execvpe behavior change in glibc 2.24?

execlp/execvp/execvpe behavior change in glibc 2.24?

[Michael Kerrisk (man-pages)]

Hello Adhemerval,

Your commit


    commit 1eb8930608705702d5746e5491bab4e4429fcb83
    Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
    Date:   Fri Jan 22 09:58:49 2016 -0200

        posix: execvpe cleanup

Appears to have caused a behavior change in execlp/execvp/execvpe,
and I am wondering whether it was intentional.

In glibc 2.23 and earlier, if PATH was undefined, then the default
path search list that was used included the current working
directory as the first entry.

      if (path == NULL)
        {
          /* There is no `PATH' in the environment.
             The default search path is the current directory
             followed by the path `confstr' returns for `_CS_PATH'.  */
          path = name + pathlen + len + 1;
          path[0] = ':';
          (void) confstr (_CS_PATH, path + 1, pathlen);
        } 

Starting in glibc-2.24, we have:

  const char *path = getenv ("PATH");
  if (!path)
    path = CS_PATH;

and

      #define CS_PATH "/bin:/usr/bin"

This excludes the CWD from the list. And testing seems to
confirm this.

I consider the change beneficial actually, since including 
the CWD in the default search path does seem risky. 
However, I see no mention in the commit message about this
behavior change, nor could I see anything in NEWS. So, I
wonder, was the change intended?

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: execlp/execvp/execvpe behavior change in glibc 2.24?

[Adhemerval Zanella]

On 20/11/2017 12:56, Michael Kerrisk (man-pages) wrote:
> Hello Adhemerval,
> 
> Your commit
> 
> 
>     commit 1eb8930608705702d5746e5491bab4e4429fcb83
>     Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>     Date:   Fri Jan 22 09:58:49 2016 -0200
> 
>         posix: execvpe cleanup
> 
> Appears to have caused a behavior change in execlp/execvp/execvpe,
> and I am wondering whether it was intentional.

I am trying to recall if this specific change was really intentional,
but nothing special come to my mind and checking review thread also
does not help much.

> 
> In glibc 2.23 and earlier, if PATH was undefined, then the default
> path search list that was used included the current working
> directory as the first entry.
> 
>       if (path == NULL)
>         {
>           /* There is no `PATH' in the environment.
>              The default search path is the current directory
>              followed by the path `confstr' returns for `_CS_PATH'.  */
>           path = name + pathlen + len + 1;
>           path[0] = ':';
>           (void) confstr (_CS_PATH, path + 1, pathlen);
>         } 
> 
> Starting in glibc-2.24, we have:
> 
>   const char *path = getenv ("PATH");
>   if (!path)
>     path = CS_PATH;
> 
> and
> 
>       #define CS_PATH "/bin:/usr/bin"
> 
> This excludes the CWD from the list. And testing seems to
> confirm this.
> 
> I consider the change beneficial actually, since including 
> the CWD in the default search path does seem risky. 
> However, I see no mention in the commit message about this
> behavior change, nor could I see anything in NEWS. So, I
> wonder, was the change intended?

Review it I also agree this seems beneficial and POSIX states
this special case is implementation-defined.  I also do not oppose
if this might characterize as compat issue and we should go back
to old behaviour.  Thoughts?  

Re: execlp/execvp/execvpe behavior change in glibc 2.24?

[Florian Weimer]

On 11/20/2017 04:54 PM, Adhemerval Zanella wrote:
> Review it I also agree this seems beneficial and POSIX states
> this special case is implementation-defined.  I also do not oppose
> if this might characterize as compat issue and we should go back
> to old behaviour.  Thoughts?

I would leave it as it is now.  The compatibility impact does not seem 
to be extreme, otherwise we'd already received a bug report.

We should update the relevant NEWS files retroactively, though.

Thanks,
Florian

Re: execlp/execvp/execvpe behavior change in glibc 2.24?

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

On Mon, Nov 20, 2017 at 03:56:18PM +0100, Michael Kerrisk (man-pages) wrote:
> Hello Adhemerval,
> 
> Your commit
> 
> 
>     commit 1eb8930608705702d5746e5491bab4e4429fcb83
>     Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>     Date:   Fri Jan 22 09:58:49 2016 -0200
> 
>         posix: execvpe cleanup
> 
> Appears to have caused a behavior change in execlp/execvp/execvpe,
> and I am wondering whether it was intentional.
> 
> In glibc 2.23 and earlier, if PATH was undefined, then the default
> path search list that was used included the current working
> directory as the first entry.
> 
>       if (path == NULL)
>         {
>           /* There is no `PATH' in the environment.
>              The default search path is the current directory
>              followed by the path `confstr' returns for `_CS_PATH'.  */
>           path = name + pathlen + len + 1;
>           path[0] = ':';
>           (void) confstr (_CS_PATH, path + 1, pathlen);
>         } 
> 
> Starting in glibc-2.24, we have:
> 
>   const char *path = getenv ("PATH");
>   if (!path)
>     path = CS_PATH;
> 
> and
> 
>       #define CS_PATH "/bin:/usr/bin"
> 
> This excludes the CWD from the list. And testing seems to
> confirm this.
> 
> I consider the change beneficial actually, since including 
> the CWD in the default search path does seem risky. 
> However, I see no mention in the commit message about this
> behavior change, nor could I see anything in NEWS. So, I
> wonder, was the change intended?

It seemed so obviously intentional at that time (it finally allowed ALT
to drop a local patch with the same effect) so I didn't manage
to point this out during review.


-- 
ldv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]