diff mbox series

realpath: Do not copy result on failure (BZ #28815)

Message ID 20220124165409.1112537-1-siddhesh@sourceware.org
State Committed
Commit 949ad78a189194048df8a253bb31d1d11d919044
Headers
Series realpath: Do not copy result on failure (BZ #28815) |

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. 24, 2022, 4:54 p.m. UTC 
  On failure, the contents of the resolved buffer passed in by the caller
to realpath are undefined.  Do not copy any partial resolution to the
buffer and also do not test resolved contents in test-canon.c.

Resolves: BZ #28815

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 stdlib/canonicalize.c | 4 ++--
 stdlib/test-canon.c   | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Adhemerval Zanella Feb. 10, 2022, 12:36 p.m. UTC | #1 
On 24/01/2022 13:54, Siddhesh Poyarekar via Libc-alpha wrote:
> On failure, the contents of the resolved buffer passed in by the caller
> to realpath are undefined.  Do not copy any partial resolution to the
> buffer and also do not test resolved contents in test-canon.c.
> 
> Resolves: BZ #28815
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  stdlib/canonicalize.c | 4 ++--
>  stdlib/test-canon.c   | 4 +++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
> index 6caed9e70e..6237a41d42 100644
> --- a/stdlib/canonicalize.c
> +++ b/stdlib/canonicalize.c
> @@ -400,11 +400,11 @@ realpath_stk (const char *name, char *resolved,
>  
>  error:
>    *dest++ = '\0';
> -  if (resolved != NULL)
> +  if (!failed && resolved != NULL)
>      {
>        if (dest - rname <= get_path_max ())
>  	rname = strcpy (resolved, rname);
> -      else if (!failed)
> +      else
>  	{
>  	  failed = true;
>  	  __set_errno (ENAMETOOLONG);
> diff --git a/stdlib/test-canon.c b/stdlib/test-canon.c
> index 185ccf4f48..2ad1218749 100644
> --- a/stdlib/test-canon.c
> +++ b/stdlib/test-canon.c
> @@ -174,7 +174,9 @@ do_test (int argc, char ** argv)
>  	  continue;
>  	}
>  
> -      if (!check_path (buf, tests[i].out ? tests[i].out : tests[i].resolved))
> +      /* Only on success verify that buf contains the result too.  */
> +      if (result != NULL
> +	  && !check_path (buf, tests[i].out ? tests[i].out : tests[i].resolved))
>  	{
>  	  printf ("%s: flunked test %d (expected resolved `%s', got `%s')\n",
>  		  argv[0], i, tests[i].out ? tests[i].out : tests[i].resolved,
diff mbox series

Patch

diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
index 6caed9e70e..6237a41d42 100644
--- a/stdlib/canonicalize.c
+++ b/stdlib/canonicalize.c
@@ -400,11 +400,11 @@  realpath_stk (const char *name, char *resolved,
 
 error:
   *dest++ = '\0';
-  if (resolved != NULL)
+  if (!failed && resolved != NULL)
     {
       if (dest - rname <= get_path_max ())
 	rname = strcpy (resolved, rname);
-      else if (!failed)
+      else
 	{
 	  failed = true;
 	  __set_errno (ENAMETOOLONG);
diff --git a/stdlib/test-canon.c b/stdlib/test-canon.c
index 185ccf4f48..2ad1218749 100644
--- a/stdlib/test-canon.c
+++ b/stdlib/test-canon.c
@@ -174,7 +174,9 @@  do_test (int argc, char ** argv)
 	  continue;
 	}
 
-      if (!check_path (buf, tests[i].out ? tests[i].out : tests[i].resolved))
+      /* Only on success verify that buf contains the result too.  */
+      if (result != NULL
+	  && !check_path (buf, tests[i].out ? tests[i].out : tests[i].resolved))
 	{
 	  printf ("%s: flunked test %d (expected resolved `%s', got `%s')\n",
 		  argv[0], i, tests[i].out ? tests[i].out : tests[i].resolved,