[PATCH] posix: execvpe: fix UMR with file > NAME_MAX [BZ #33627]

[Collin Funk]

Carlos O'Donell <carlos@redhat.com> writes:

> On 11/14/25 1:37 PM, Collin Funk wrote:
>> Pádraig Brady <P@draigBrady.com> writes:
>> 
>>> * posix/execvpe.c (__execvpe_common): Since strnlen doesn't inspect
>>> beyond NAME_MAX and NAME_MAX does not cover the NUL, we need
>>> to explicitly check for the NUL.  I.e. the existing check for,
>>> file_len-1 > NAME_MAX, was never true.  This check is required
>>> so that we're guaranteed that file_len includes the NUL, as we
>>> depend on that in the following memcpy to properly terminate
>>> the file buffer passed to execve().  Otherwise that call will trigger
>>> UMR when inspecting the passed file, which can be seen with valgrind.
>>> Note returning ENAMETOOLONG early here for FILE names > NAME_MAX
>>> will also avoid redundant processing of ENAMETOOLONG on each entry
>>> in $PATH, after the change in [BZ #33626] is applied.
>>> ---
>>>   posix/execvpe.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/posix/execvpe.c b/posix/execvpe.c
>>> index 0fb09b9631..433b5bb0b3 100644
>>> --- a/posix/execvpe.c
>>> +++ b/posix/execvpe.c
>>> @@ -98,8 +98,9 @@ __execvpe_common (const char *file, char *const argv[], char *const envp[],
>>>     size_t file_len = __strnlen (file, NAME_MAX) + 1;
>>>     size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
>>>   -  /* NAME_MAX does not include the terminating null character.
>>> */
>>> -  if ((file_len - 1 > NAME_MAX)
>>> +  /* NAME_MAX does not include the terminating NUL character.
>>> +     The following check ensures FILE is NUL terminated.  */
>>> +  if ((file_len - 1 == NAME_MAX && file[NAME_MAX] != '\0')
>>>         || !__libc_alloca_cutoff (path_len + file_len + 1))
>>>       {
>>>         errno = ENAMETOOLONG;
>> Nice catch. Thanks!
>> Reviewed-by: Collin Funk <collin.funk1@gmail.com>
>
> May you please help Padraig get this pushed by committing on their behalf?

I thought it would need approval from a subsystem maintainer, since I
only have write after consensus.

Anyways, I pushed the change now. No need for DCO sign-off since it is
only a few lines.

Pádraig, I'll review your other change in a bit. I need to do some
reading to understand Bruno's disagreement.

Thanks,
Collin