On 04/08/2024 22:48, Jon Turney wrote:
Supress new use-after-free warnings about realloc(), seen with gcc 12, e.g.:
In function ‘void rebase_locale_buf(const void*, const void*, const char*,
const char*, const char*)’,
inlined from ‘int __set_lc_time_from_win(const char*, const lc_time_T*,
lc_time_T*, char**, wctomb_p, const char*)’ at
../../../../src/winsup/cygwin/nlsfuncs.cc:705:25:
../../../../src/winsup/cygwin/nlsfuncs.cc:338:24: error: pointer
‘new_lc_time_buf’ may be used after ‘void* realloc(void*, size_t)’
[-Werror=use-after-free]
338 | *ptrs += newbase - oldbase;
|^
../../../../src/winsup/cygwin/nlsfuncs.cc: In function ‘int
__set_lc_time_from_win(const char*, const lc_time_T*, lc_time_T*, char**,
wctomb_p, const char*)’:
../../../../src/winsup/cygwin/nlsfuncs.cc:699:44: note: call to ‘void*
realloc(void*, size_t)’ here
699 | char *tmp = (char *) realloc (new_lc_time_buf, len);
We do some calculations using the pointer passed to realloc(), but do
not not dereference it, so this seems safe?
Since this is less than ideal, here's the version where we explicitly
malloc() the new buffer, adjust things, then free() the old buffer.
This is all quite hairy, though, and I have no idea how to begin to test
this, so if you have some pointers to share, that would be good.
From 8f90b214c67db5fec42a7e9b51d5c2a8ec4d1510 Mon Sep 17 00:00:00 2001
From: Jon Turney
Date: Sun, 4 Aug 2024 17:15:50 +0100
Subject: [PATCH] Cygwin: Avoid use-after-free warnings in
__set_lc_time_from_win() etc.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Rewrite to avoid new use-after-free warnings about realloc(), seen with
gcc 12, e.g.:
> In function ‘void rebase_locale_buf(const void*, const void*, const char*,
> const char*, const char*)’,
> inlined from ‘int __set_lc_time_from_win(const char*, const lc_time_T*,
> lc_time_T*, char**, wctomb_p, const char*)’ at
> ../../../../src/winsup/cygwin/nlsfuncs.cc:705:25:
> ../../../../src/winsup/cygwin/nlsfuncs.cc:338:24: error: pointer
> ‘new_lc_time_buf’ may be used after ‘void* realloc(void*, size_t)’
> [-Werror=use-after-free]
> 338 | *ptrs += newbase - oldbase;
> |^
> ../../../../src/winsup/cygwin/nlsfuncs.cc: In function ‘int
> __set_lc_time_from_win(const char*, const lc_time_T*, lc_time_T*, char**,
> wctomb_p, const char*)’:
> ../../../../src/winsup/cygwin/nlsfuncs.cc:699:44: note: call to ‘void*
> realloc(void*, size_t)’ here
> 699 | char *tmp = (char *) realloc (new_lc_time_buf, len);
Technically, it's UB to later refer to the realloced pointer (even just
for offset computations, without deferencing it), so switch to using
malloc() to create the resized copy.
Signed-off-by: Jon Turney
---
winsup/cygwin/nlsfuncs.cc | 95 +--
1 file changed, 41 insertions(+), 54 deletions(-)
diff --git a/winsup/cygwin/nlsfuncs.cc b/winsup/cygwin/nlsfuncs.cc
index b32fecc8e..1b33cb79f 100644
--- a/winsup/cygwin/nlsfuncs.cc
+++ b/winsup/cygwin/nlsfuncs.cc
@@ -319,8 +319,7 @@ locale_cmp (const void *a, const void *b)
return strcmp (*la, *lb);
}
-/* Helper function to workaround reallocs which move blocks even if they
shrink.
- Cygwin's realloc is not doing this, but tcsh's, for instance. All lc_foo
+/* Helper function to adjust pointers inside a lc_foo buffer. All lc_foo
structures consist entirely of pointers so they are practically pointer
arrays. What we do here is just treat the lc_foo pointers as char ** and
rebase all char * pointers within, up to the given size of the structure. */
@@ -334,6 +333,28 @@ rebase_locale_buf (const void *ptrv, const void *ptrvend,
const char *newbase,
*ptrs += newbase - oldbase;
}
+/* Helper function to shrink a lc_foo buffer, adjusting pointers */
+static int
+shrink_local_buf (const void *ptrv, const void *ptrvend,
+ char *oldbase, const char *oldend,
+ char **result)
+{
+ size_t minsize = oldend - oldbase;
+ char *tmp = (char *) malloc (minsize);
+ if (!tmp)
+{
+ free (oldbase);
+ return -1;
+}
+
+ memcpy (tmp, oldbase, minsize);
+ rebase_locale_buf (ptrv, ptrvend, tmp, oldbase, oldend);
+ free (oldbase);
+
+ *result = tmp;
+ return 1;
+}
+
static wchar_t *
__getlocaleinfo (wchar_t *loc, LCTYPE type, char **ptr, size_t size)
{
@@ -687,19 +708,20 @@ __set_lc_time_from_win (const char *name,
len += (wcslen (era->era_t_fmt) + 1) * sizeof (wchar_t);
len += (wcslen (era->alt_digits) + 1) * sizeof (wchar_t);
- /* Make sure data fits into the buffer */
+ /* If necessary, grow the buffer to ensure data fits into it */
if (lc_time_ptr + len > lc_time_end)
{
len = lc_time_ptr + len - new_lc_time_buf;
- char *tmp = (char *) realloc (new_lc_time_buf, l
