diff mbox series

tst-realpath-toolong: Fix hurd build

Message ID 20220122144523.2221584-1-siddhesh@sourceware.org
State Superseded
Headers show
Series tst-realpath-toolong: Fix hurd build | expand

Checks

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

Commit Message

Siddhesh Poyarekar Jan. 22, 2022, 2:45 p.m. UTC
We don't really need a bigger buffer for realpath since it should fail
and return NULL.  In the bug too, the buffer itself is not accessed; it
is in fact left untouched.  Drop the PATH_MAX use and pass a single char
address.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 stdlib/tst-realpath-toolong.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Samuel Thibault Jan. 23, 2022, 12:36 a.m. UTC | #1
Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit:
> We don't really need a bigger buffer for realpath since it should fail
> and return NULL.  In the bug too, the buffer itself is not accessed; it
> is in fact left untouched.  Drop the PATH_MAX use and pass a single char
> address.

? realpath assumes that the passed buffer is PATH_MAX-long. When
PATH_MAX is not defined, calling it with a buffer is essentially
undefined. Better just pass NULL.

> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> ---
>  stdlib/tst-realpath-toolong.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c
> index 8bed772460..ed84787a32 100644
> --- a/stdlib/tst-realpath-toolong.c
> +++ b/stdlib/tst-realpath-toolong.c
> @@ -34,8 +34,8 @@ do_test (void)
>  {
>    char *base = support_create_and_chdir_toolong_temp_directory (BASENAME);
>  
> -  char buf[PATH_MAX + 1];
> -  const char *res = realpath (".", buf);
> +  char buf;
> +  const char *res = realpath (".", &buf);
>  
>    /* canonicalize.c states that if the real path is >= PATH_MAX, then
>       realpath returns NULL and sets ENAMETOOLONG.  */
> -- 
> 2.34.1
>
Siddhesh Poyarekar Jan. 23, 2022, 3:19 p.m. UTC | #2
On 23/01/2022 06:06, Samuel Thibault wrote:
> Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit:
>> We don't really need a bigger buffer for realpath since it should fail
>> and return NULL.  In the bug too, the buffer itself is not accessed; it
>> is in fact left untouched.  Drop the PATH_MAX use and pass a single char
>> address.
> 
> ? realpath assumes that the passed buffer is PATH_MAX-long. When
> PATH_MAX is not defined, calling it with a buffer is essentially
> undefined. Better just pass NULL.

Passing NULL doesn't reproduce the problem because realpath just 
allocates enough to accommodate the return, even when it exceeds 
PATH_MAX.  It only applies when a non-NULL buffer is supplied.

Would you prefer it if I defined PATH_MAX on hurd then, something like:

#ifndef PATH_MAX
# define PATH_MAX 1024
#endif

or do you prefer a more accurate path_max value using pathconf()?

The former will be a simpler fix, the latter will be best served by a 
get_path_max support function, which will be more elaborate but 
accurate.  I'm happy to do either.

Thanks,
Siddhesh
Samuel Thibault Jan. 23, 2022, 3:33 p.m. UTC | #3
Siddhesh Poyarekar, le dim. 23 janv. 2022 20:49:48 +0530, a ecrit:
> On 23/01/2022 06:06, Samuel Thibault wrote:
> > Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit:
> > > We don't really need a bigger buffer for realpath since it should fail
> > > and return NULL.  In the bug too, the buffer itself is not accessed; it
> > > is in fact left untouched.  Drop the PATH_MAX use and pass a single char
> > > address.
> > 
> > ? realpath assumes that the passed buffer is PATH_MAX-long. When
> > PATH_MAX is not defined, calling it with a buffer is essentially
> > undefined. Better just pass NULL.
> 
> Passing NULL doesn't reproduce the problem because realpath just allocates
> enough to accommodate the return, even when it exceeds PATH_MAX.

Ah, ok, sorry I didn't check the whole story.

> Would you prefer it if I defined PATH_MAX on hurd then, something like:
> 
> #ifndef PATH_MAX
> # define PATH_MAX 1024
> #endif
> 
> or do you prefer a more accurate path_max value using pathconf()?

Better use the accurate from pathconf() ; that being said it will return
-1 on the ext2fs filesystem, so we'll have to resort to a hardcoded
limit in that case anyway. I'm fine with just setting PATH_MAX by hand
here.

Samuel
diff mbox series

Patch

diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c
index 8bed772460..ed84787a32 100644
--- a/stdlib/tst-realpath-toolong.c
+++ b/stdlib/tst-realpath-toolong.c
@@ -34,8 +34,8 @@  do_test (void)
 {
   char *base = support_create_and_chdir_toolong_temp_directory (BASENAME);
 
-  char buf[PATH_MAX + 1];
-  const char *res = realpath (".", buf);
+  char buf;
+  const char *res = realpath (".", &buf);
 
   /* canonicalize.c states that if the real path is >= PATH_MAX, then
      realpath returns NULL and sets ENAMETOOLONG.  */