public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* Fix Hurd getcwd build with GCC >= 13
@ 2023-04-26 17:14 Joseph Myers
  2023-04-26 23:29 ` Samuel Thibault
  0 siblings, 1 reply; 3+ messages in thread
From: Joseph Myers @ 2023-04-26 17:14 UTC (permalink / raw)
  To: libc-alpha; +Cc: samuel.thibault

The build of glibc for i686-gnu has been failing for a while with GCC
mainline / GCC 13:

../sysdeps/mach/hurd/getcwd.c: In function '__hurd_canonicalize_directory_name_internal':
../sysdeps/mach/hurd/getcwd.c:242:48: error: pointer 'file_name' may be used after 'realloc' [-Werror=use-after-free]
  242 |                   file_namep = &buf[file_namep - file_name + size / 2];
      |                                     ~~~~~~~~~~~^~~~~~~~~~~
../sysdeps/mach/hurd/getcwd.c:236:25: note: call to 'realloc' here
  236 |                   buf = realloc (file_name, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~

This appears to be a genuine bug; fix by doing the subtraction before
the reallocation makes the pointer invalid for arithmetic.

Tested with build-many-glibcs.py for i686-gnu.

diff --git a/sysdeps/mach/hurd/getcwd.c b/sysdeps/mach/hurd/getcwd.c
index f24b35b380..cd3aedd9cd 100644
--- a/sysdeps/mach/hurd/getcwd.c
+++ b/sysdeps/mach/hurd/getcwd.c
@@ -222,8 +222,9 @@ __hurd_canonicalize_directory_name_internal (file_t thisdir,
       found:
 	{
 	  /* Prepend the directory name just discovered.  */
+	  size_t offset = file_namep - file_name;
 
-	  if (file_namep - file_name < d->d_namlen + 1)
+	  if (offset < d->d_namlen + 1)
 	    {
 	      if (orig_size > 0)
 		{
@@ -239,7 +240,7 @@ __hurd_canonicalize_directory_name_internal (file_t thisdir,
 		      free (file_name);
 		      return NULL;
 		    }
-		  file_namep = &buf[file_namep - file_name + size / 2];
+		  file_namep = &buf[offset + size / 2];
 		  file_name = buf;
 		  /* Move current contents up to the end of the buffer.
 		     This is guaranteed to be non-overlapping.  */

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Fix Hurd getcwd build with GCC >= 13
  2023-04-26 17:14 Fix Hurd getcwd build with GCC >= 13 Joseph Myers
@ 2023-04-26 23:29 ` Samuel Thibault
  2023-04-26 23:34   ` Joseph Myers
  0 siblings, 1 reply; 3+ messages in thread
From: Samuel Thibault @ 2023-04-26 23:29 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha

Joseph Myers, le mer. 26 avril 2023 17:14:18 +0000, a ecrit:
> The build of glibc for i686-gnu has been failing for a while with GCC
> mainline / GCC 13:
> 
> ../sysdeps/mach/hurd/getcwd.c: In function '__hurd_canonicalize_directory_name_internal':
> ../sysdeps/mach/hurd/getcwd.c:242:48: error: pointer 'file_name' may be used after 'realloc' [-Werror=use-after-free]
>   242 |                   file_namep = &buf[file_namep - file_name + size / 2];
>       |                                     ~~~~~~~~~~~^~~~~~~~~~~
> ../sysdeps/mach/hurd/getcwd.c:236:25: note: call to 'realloc' here
>   236 |                   buf = realloc (file_name, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This appears to be a genuine bug; fix by doing the subtraction before
> the reallocation makes the pointer invalid for arithmetic.

Well, it's actually not a genuine bug: the values of the file_namep
and file_name pointers are still coherent, so the subtraction is still
valid. But better please gcc 13 indeed, I have pushed it, thanks!

> Tested with build-many-glibcs.py for i686-gnu.
> 
> diff --git a/sysdeps/mach/hurd/getcwd.c b/sysdeps/mach/hurd/getcwd.c
> index f24b35b380..cd3aedd9cd 100644
> --- a/sysdeps/mach/hurd/getcwd.c
> +++ b/sysdeps/mach/hurd/getcwd.c
> @@ -222,8 +222,9 @@ __hurd_canonicalize_directory_name_internal (file_t thisdir,
>        found:
>  	{
>  	  /* Prepend the directory name just discovered.  */
> +	  size_t offset = file_namep - file_name;
>  
> -	  if (file_namep - file_name < d->d_namlen + 1)
> +	  if (offset < d->d_namlen + 1)
>  	    {
>  	      if (orig_size > 0)
>  		{
> @@ -239,7 +240,7 @@ __hurd_canonicalize_directory_name_internal (file_t thisdir,
>  		      free (file_name);
>  		      return NULL;
>  		    }
> -		  file_namep = &buf[file_namep - file_name + size / 2];
> +		  file_namep = &buf[offset + size / 2];
>  		  file_name = buf;
>  		  /* Move current contents up to the end of the buffer.
>  		     This is guaranteed to be non-overlapping.  */
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Fix Hurd getcwd build with GCC >= 13
  2023-04-26 23:29 ` Samuel Thibault
@ 2023-04-26 23:34   ` Joseph Myers
  0 siblings, 0 replies; 3+ messages in thread
From: Joseph Myers @ 2023-04-26 23:34 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: libc-alpha

On Thu, 27 Apr 2023, Samuel Thibault wrote:

> Well, it's actually not a genuine bug: the values of the file_namep
> and file_name pointers are still coherent, so the subtraction is still
> valid. But better please gcc 13 indeed, I have pushed it, thanks!

It's a bug because "If a pointer value is used in an evaluation after the 
object the pointer points to (or just past) reaches the end of its 
lifetime, the behavior is undefined.  The representation of a pointer 
object becomes indeterminate when the object the pointer points to (or 
just past) reaches the end of its lifetime." (N3096 wording, but the 
principle that you can't use pointers to deallocated memory, even without 
dereferencing them, dates back a lot further).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-04-26 23:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-26 17:14 Fix Hurd getcwd build with GCC >= 13 Joseph Myers
2023-04-26 23:29 ` Samuel Thibault
2023-04-26 23:34   ` Joseph Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).