[newlib-cygwin/cygwin-3_5-branch] Cygwin: Avoid use-after-free warnings in __set_lc_time_from_win() etc.
Corinna Vinschen
corinna@sourceware.org
Thu Aug 22 20:38:00 GMT 2024
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=65e3e2fd0593edbbe9a6757c188d9e1f39faa2f9
commit 65e3e2fd0593edbbe9a6757c188d9e1f39faa2f9
Author: Jon Turney <jon.turney@dronecode.org.uk>
AuthorDate: Sun Aug 4 17:15:50 2024 +0100
Commit: Corinna Vinschen <corinna@vinschen.de>
CommitDate: Thu Aug 22 19:58:49 2024 +0200
Cygwin: Avoid use-after-free warnings in __set_lc_time_from_win() etc.
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 <jon.turney@dronecode.org.uk>
Diff:
---
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 b32fecc8ec33..f57465a4f2f3 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 an 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 an lc_foo buffer, adjusting pointers */
+static int
+shrink_locale_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, len);
+ char *tmp = (char *) malloc (len);
if (!tmp)
era = NULL;
else
{
- if (tmp != new_lc_time_buf)
- rebase_locale_buf (_time_locale, _time_locale + 1, tmp,
- new_lc_time_buf, lc_time_ptr);
+ memcpy (tmp, new_lc_time_buf, MAX_TIME_BUFFER_SIZE);
+ rebase_locale_buf (_time_locale, _time_locale + 1, tmp,
+ new_lc_time_buf, lc_time_ptr);
lc_time_ptr = tmp + (lc_time_ptr - new_lc_time_buf);
+ free(new_lc_time_buf);
new_lc_time_buf = tmp;
lc_time_end = new_lc_time_buf + len;
}
@@ -752,17 +774,9 @@ __set_lc_time_from_win (const char *name,
}
}
- char *tmp = (char *) realloc (new_lc_time_buf, lc_time_ptr - new_lc_time_buf);
- if (!tmp)
- {
- free (new_lc_time_buf);
- return -1;
- }
- if (tmp != new_lc_time_buf)
- rebase_locale_buf (_time_locale, _time_locale + 1, tmp,
- new_lc_time_buf, lc_time_ptr);
- *lc_time_buf = tmp;
- return 1;
+ return shrink_locale_buf(_time_locale, _time_locale + 1,
+ new_lc_time_buf, lc_time_ptr,
+ lc_time_buf);
}
/* Called from newlib's setlocale() via __ctype_load_locale() if category
@@ -824,18 +838,9 @@ __set_lc_ctype_from_win (const char *name,
}
}
- char *tmp = (char *) realloc (new_lc_ctype_buf,
- lc_ctype_ptr - new_lc_ctype_buf);
- if (!tmp)
- {
- free (new_lc_ctype_buf);
- return -1;
- }
- if (tmp != new_lc_ctype_buf)
- rebase_locale_buf (_ctype_locale, _ctype_locale + 1, tmp,
- new_lc_ctype_buf, lc_ctype_ptr);
- *lc_ctype_buf = tmp;
- return 1;
+ return shrink_locale_buf(_ctype_locale, _ctype_locale + 1,
+ new_lc_ctype_buf, lc_ctype_ptr,
+ lc_ctype_buf);
}
/* Called from newlib's setlocale() via __numeric_load_locale() if category
@@ -900,18 +905,9 @@ __set_lc_numeric_from_win (const char *name,
_numeric_locale->codeset = lc_numeric_ptr;
lc_numeric_ptr = stpcpy (lc_numeric_ptr, charset) + 1;
- char *tmp = (char *) realloc (new_lc_numeric_buf,
- lc_numeric_ptr - new_lc_numeric_buf);
- if (!tmp)
- {
- free (new_lc_numeric_buf);
- return -1;
- }
- if (tmp != new_lc_numeric_buf)
- rebase_locale_buf (_numeric_locale, _numeric_locale + 1, tmp,
- new_lc_numeric_buf, lc_numeric_ptr);
- *lc_numeric_buf = tmp;
- return 1;
+ return shrink_locale_buf(_numeric_locale, _numeric_locale + 1,
+ new_lc_numeric_buf, lc_numeric_ptr,
+ lc_numeric_buf);
}
/* Called from newlib's setlocale() via __monetary_load_locale() if category
@@ -1039,18 +1035,9 @@ __set_lc_monetary_from_win (const char *name,
_monetary_locale->codeset = lc_monetary_ptr;
lc_monetary_ptr = stpcpy (lc_monetary_ptr, charset) + 1;
- char *tmp = (char *) realloc (new_lc_monetary_buf,
- lc_monetary_ptr - new_lc_monetary_buf);
- if (!tmp)
- {
- free (new_lc_monetary_buf);
- return -1;
- }
- if (tmp != new_lc_monetary_buf)
- rebase_locale_buf (_monetary_locale, _monetary_locale + 1, tmp,
- new_lc_monetary_buf, lc_monetary_ptr);
- *lc_monetary_buf = tmp;
- return 1;
+ return shrink_locale_buf(_monetary_locale, _monetary_locale + 1,
+ new_lc_monetary_buf, lc_monetary_ptr,
+ lc_monetary_buf);
}
extern "C" int
More information about the Cygwin-cvs
mailing list