pgsql: Fix memory leakage in ICU encoding conversion, and other code re

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Fix memory leakage in ICU encoding conversion, and other code re
Date: 2017-06-23 16:22:12
Message-ID: E1dORLY-0005DJ-Ib@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

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

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/b6159202c99d4021fb078cede90b26f94883143d

Modified Files
--------------
src/backend/commands/collationcmds.c | 15 ++++++++++++---
src/backend/utils/adt/formatting.c | 3 +++
src/backend/utils/adt/pg_locale.c | 35 ++++++++++++++++++++++++++++++-----
src/backend/utils/adt/varlena.c | 16 ++++++++++++++--
src/include/utils/pg_locale.h | 2 +-
5 files changed, 60 insertions(+), 11 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Simon Riggs 2017-06-23 17:17:37 pgsql: Minor corrections to high availability docs
Previous Message Tom Lane 2017-06-23 15:03:10 pgsql: Add testing to detect errors of omission in "pin" dependency cre