From 4eb9397b6ccc725820143fbdf5feb3f62306440b Mon Sep 17 00:00:00 2001 From: Jon Turney Date: Sun, 4 Aug 2024 17:15:50 +0100 Subject: 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 b32fecc..f57465a 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 -- cgit v1.1