Skip to content

Commit

Permalink
Cygwin: Avoid use-after-free warnings in __set_lc_time_from_win() etc.
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@jon-turney
jon-turney committed Aug 13, 2024
1 parent 80e7ed9 commit 4eb9397
Showing 1 changed file with 41 additions and 54 deletions.
95 changes: 41 additions & 54 deletions winsup/cygwin/nlsfuncs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4eb9397

Please sign in to comment.