Message ID | 18587337-7815-4056-ebd0-724df262d591@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | Fix Hurd getcwd build with GCC >= 13 | expand |
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
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).
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. */