database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-25 01:02:05 +03:00
Code Activity

Fix memory leakage in ICU encoding conversion, and other code review.

Browse Source 
Callers of icu_to_uchar() neglected to pfree the result string when done
with it.  This results in catastrophic memory leaks in varstr_cmp(),
because of our prevailing assumption that btree comparison functions don't
leak memory.  For safety, make all the call sites clean up leaks, though
I suspect that we could get away without it in formatting.c.  I audited
callers of icu_from_uchar() as well, but found no places that seemed to
have a comparable issue.

Add function API specifications for icu_to_uchar() and icu_from_uchar();
the lack of any thought-through specification is perhaps not unrelated
to the existence of this bug in the first place.  Fix icu_to_uchar()
to guarantee a nul-terminated result; although no existing caller appears
to care, the fact that it would have been nul-terminated except in
extreme corner cases seems ideally designed to bite someone on the rear
someday.  Fix ucnv_fromUChars() destCapacity argument --- in the worst
case, that could perhaps have led to a non-nul-terminated result, too.
Fix icu_from_uchar() to have a more reasonable definition of the function
result --- no callers are actually paying attention, so this isn't a live
bug, but it's certainly sloppily designed.  Const-ify icu_from_uchar()'s
input string for consistency.

That is not the end of what needs to be done to these functions, but
it's as much as I have the patience for right now.

Discussion: https://postgr.es/m/1955.1498181798@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2017-06-23 12:22:06 -04:00
parent 8be8510cf8
commit b6159202c9
5 changed files with 60 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/backend/utils/adt/varlena.c
View File

@ -1569,6 +1569,9 @@ varstr_cmp(char *arg1, int len1, char *arg2, int len2, Oid collid)
result = ucol_strcoll(mylocale->info.icu.ucol,
uchar1, ulen1,
uchar2, ulen2);
pfree(uchar1);
pfree(uchar2);
}
#else /* not USE_ICU */
/* shouldn't happen */
@ -2155,6 +2158,9 @@ varstrfastcmp_locale(Datum x, Datum y, SortSupport ssup)
result = ucol_strcoll(sss->locale->info.icu.ucol,
uchar1, ulen1,
uchar2, ulen2);
pfree(uchar1);
pfree(uchar2);
}
#else /* not USE_ICU */
/* shouldn't happen */
@ -2279,7 +2285,7 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
Size bsize;
#ifdef USE_ICU
int32_t ulen = -1;
UChar *uchar;
UChar *uchar = NULL;
#endif
/*
@ -2354,7 +2360,8 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
&status);
if (U_FAILURE(status))
ereport(ERROR,
(errmsg("sort key generation failed: %s", u_errorName(status))));
(errmsg("sort key generation failed: %s",
u_errorName(status))));
}
else
bsize = ucol_getSortKey(sss->locale->info.icu.ucol,
@ -2394,6 +2401,11 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
* okay. See remarks on bytea case above.)
*/
memcpy(pres, sss->buf2, Min(sizeof(Datum), bsize));
#ifdef USE_ICU
if (uchar)
pfree(uchar);
#endif
}
/*