diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-06-23 12:22:06 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-06-23 12:22:06 -0400 |
| commit | b6159202c99d4021fb078cede90b26f94883143d (patch) | |
| tree | b1c0e32f5fc3a1a920a51c998cead273d9098720 /src/backend/utils/adt/varlena.c | |
| parent | 8be8510cf89d4e150816941029d7cdddfe9aa474 (diff) | |
Fix memory leakage in ICU encoding conversion, and other code review.
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
Diffstat (limited to 'src/backend/utils/adt/varlena.c')
| -rw-r--r-- | src/backend/utils/adt/varlena.c | 16 |
1 files changed, 14 insertions, 2 deletions
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 2407394b068..ebfb823fb8c 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -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 } /* |