posix: execvpe: skip $PATH components that are too long

Message ID 20251108155128.1758950-1-P@draigBrady.com (mailing list archive)
State Changes Requested
Headers
Series posix: execvpe: skip $PATH components that are too long |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686

Commit Message

Pádraig Brady Nov. 8, 2025, 3:51 p.m. UTC
  * posix/execvpe.c (__execvpe_common): Rather than error out
with ENAMETOOLONG, just ignore and try the next path.
Note we know the FILE length is <= NAME_MAX, so the ENAMETOOLONG
almost certainly pertains to the current $PATH entry.
---
 posix/execvpe.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Adhemerval Zanella Netto Nov. 10, 2025, 12:20 p.m. UTC | #1
On 08/11/25 16:51, Pádraig Brady wrote:
> * posix/execvpe.c (__execvpe_common): Rather than error out
> with ENAMETOOLONG, just ignore and try the next path.
> Note we know the FILE length is <= NAME_MAX, so the ENAMETOOLONG
> almost certainly pertains to the current $PATH entry.
> ---
>  posix/execvpe.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/posix/execvpe.c b/posix/execvpe.c
> index c139dfe8fd..0fb09b9631 100644
> --- a/posix/execvpe.c
> +++ b/posix/execvpe.c
> @@ -156,6 +156,9 @@ __execvpe_common (const char *file, char *const argv[], char *const envp[],
>  	  /* Those errors indicate the file is missing or not executable
>  	     by us, in which case we want to just try the next path
>  	     directory.  */
> +	  case ENAMETOOLONG:
> +	  /* We've already verified that the FILE length is < NAME_MAX,
> +	     so this implies a path component is too long, so skip it.  */
>  	  case ENODEV:
>  	  case ETIMEDOUT:
>  	  /* Some strange filesystems like AFS return even

I think if Paul's latest comment holds that this is indeed an issue, we should
add a bug report and a regression test for this issue.
  
Pádraig Brady Nov. 13, 2025, 6:31 p.m. UTC | #2
Adhemerval Zanella Netto wrote:
On 08/11/25 16:51, Pádraig Brady wrote:
 > > * posix/execvpe.c (__execvpe_common): Rather than error out
 > > with ENAMETOOLONG, just ignore and try the next path.
 > > Note we know the FILE length is <= NAME_MAX, so the ENAMETOOLONG
 > > almost certainly pertains to the current $PATH entry.
 > > ---
 > >  posix/execvpe.c | 3 +++
 > >  1 file changed, 3 insertions(+)
 > >
 > > diff --git a/posix/execvpe.c b/posix/execvpe.c
 > > index c139dfe8fd..0fb09b9631 100644
 > > --- a/posix/execvpe.c
 > > +++ b/posix/execvpe.c
 > > @@ -156,6 +156,9 @@ __execvpe_common (const char *file, char *const argv[], char *const envp[],
 > >  	  /* Those errors indicate the file is missing or not executable
 > >  	     by us, in which case we want to just try the next path
 > >  	     directory.  */
 > > +	  case ENAMETOOLONG:
 > > +	  /* We've already verified that the FILE length is < NAME_MAX,
 > > +	     so this implies a path component is too long, so skip it.  */
 > >  	  case ENODEV:
 > >  	  case ETIMEDOUT:
 > >  	  /* Some strange filesystems like AFS return even
 >
 > I think if Paul's latest comment holds that this is indeed an issue, we should
 > add a bug report and a regression test for this issue.

Cool. I've created https://sourceware.org/PR33626
and attached an updated patch to include a test case
(also attached here).

cheers,
Padraig
  
Collin Funk Nov. 13, 2025, 6:49 p.m. UTC | #3
Hi Pádraig,

Pádraig Brady <P@draigBrady.com> writes:

> Adhemerval Zanella Netto wrote:
> On 08/11/25 16:51, Pádraig Brady wrote:
>> > * posix/execvpe.c (__execvpe_common): Rather than error out
>> > with ENAMETOOLONG, just ignore and try the next path.
>> > Note we know the FILE length is <= NAME_MAX, so the ENAMETOOLONG
>> > almost certainly pertains to the current $PATH entry.
>> > ---
>> >  posix/execvpe.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/posix/execvpe.c b/posix/execvpe.c
>> > index c139dfe8fd..0fb09b9631 100644
>> > --- a/posix/execvpe.c
>> > +++ b/posix/execvpe.c
>> > @@ -156,6 +156,9 @@ __execvpe_common (const char *file, char *const argv[], char *const envp[],
>> >  	  /* Those errors indicate the file is missing or not executable
>> >  	     by us, in which case we want to just try the next path
>> >  	     directory.  */
>> > +	  case ENAMETOOLONG:
>> > +	  /* We've already verified that the FILE length is < NAME_MAX,
>> > +	     so this implies a path component is too long, so skip it.  */
>> >  	  case ENODEV:
>> >  	  case ETIMEDOUT:
>> >  	  /* Some strange filesystems like AFS return even
>>
>> I think if Paul's latest comment holds that this is indeed an issue, we should
>> add a bug report and a regression test for this issue.
>
> Cool. I've created https://sourceware.org/PR33626
> and attached an updated patch to include a test case
> (also attached here).

For future reference, patches on the bug tracker tend to get lost. You
also sent it here, so no big deal.

> diff --git a/posix/tst-execvpe7.c b/posix/tst-execvpe7.c
> new file mode 100644
> index 0000000000..7df77da8e0
> --- /dev/null
> +++ b/posix/tst-execvpe7.c
> @@ -0,0 +1,66 @@
> +/* Check execvpe script argument handling.
> +   Copyright (C) 2016-2025 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <limits.h>
> +#include <support/xstdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/param.h>
> +#include <sys/stat.h>
> +
> +
> +static void do_prepare (void);
> +#define PREPARE(argc, argv) do_prepare ()
> +static int do_test (void);
> +#define TEST_FUNCTION do_test ()
> +
> +#include "../test-skeleton.c"
> +

I assume you wrote this referencing an old test that used
test-skeleton.c. New tests use "#include <support/test-driver.c>".

Could you please adjust it to use that instead? If you would like to
look at an example for reference, I wrote posix/tst-glob-bz30635.c
recently.

It would be nice to port the old tests to use support/test-driver.c to
avoid confusion. But I am not sure if there are any plans to.

Collin
  
Pádraig Brady Nov. 13, 2025, 10:14 p.m. UTC | #4
Collin Funk <collin.funk1@gmail.com> writes:
 > I assume you wrote this referencing an old test that used
 > test-skeleton.c. New tests use "#include <support/test-driver.c>".
 >
 > Could you please adjust it to use that instead?

I've updated that and attached here and to the bug report.

BTW there is also a related issue (with patch) I reported at:
https://sourceware.org/PR33627

thanks,
Padraig.
  

Patch

diff --git a/posix/execvpe.c b/posix/execvpe.c
index c139dfe8fd..0fb09b9631 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -156,6 +156,9 @@  __execvpe_common (const char *file, char *const argv[], char *const envp[],
 	  /* Those errors indicate the file is missing or not executable
 	     by us, in which case we want to just try the next path
 	     directory.  */
+	  case ENAMETOOLONG:
+	  /* We've already verified that the FILE length is < NAME_MAX,
+	     so this implies a path component is too long, so skip it.  */
 	  case ENODEV:
 	  case ETIMEDOUT:
 	  /* Some strange filesystems like AFS return even