Commits b181a919 and arguably c79b6413 added bugs to
bttext_abbrev_convert() in the process of fixing some others. In the
master branch, bttext_abbrev_convert() can leak memory when the C
locale happens to be used and we must detoast, which is unacceptable
for about the same reason that it's unacceptable for a standard B-Tree
comparator routine. There could also be a use-after-free issue for
large strings that are detoasted, because bttext_abbrev_convert()
hashes memory which might already be freed (when hashing the
authoritative value).
Attached patch fixes these issues.
As we all know, the state of automated testing is pretty lamentable.
This is the kind of thing that we could catch more easily in the
future if better infrastructure were in place. I caught this by
eyeballing bttext_abbrev_convert() with slightly fresher eyes than the
last time I looked.
--
Peter Geoghegan
From 9aa381b1c136b9c4228b0b965c38fc62234ba251 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <peter.geoghega...@gmail.com>
Date: Mon, 29 Jun 2015 16:21:16 -0700
Subject: [PATCH] Fix memory management bugs in text abbreviation
Make sure that a pfree() of an unpacked Datum value (from a toasted
Datum) always occurs, including when a text sort uses the "C" locale.
Free unpacked value memory at the latest possible opportunity, ensuring
that it is not spuriously referenced after pfree().
---
src/backend/utils/adt/varlena.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 779729d..2fbbf54 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2034,13 +2034,9 @@ bttext_abbrev_convert(Datum original, SortSupport ssup)
}
/* Just like strcoll(), strxfrm() expects a NUL-terminated string */
- memcpy(tss->buf1, VARDATA_ANY(authoritative), len);
+ memcpy(tss->buf1, authoritative_data, len);
tss->buf1[len] = '\0';
- /* Don't leak memory here */
- if (PointerGetDatum(authoritative) != original)
- pfree(authoritative);
-
for (;;)
{
#ifdef HAVE_LOCALE_T
@@ -2108,6 +2104,10 @@ bttext_abbrev_convert(Datum original, SortSupport ssup)
addHyperLogLog(&tss->abbr_card, hash);
+ /* Don't leak memory here */
+ if (PointerGetDatum(authoritative) != original)
+ pfree(authoritative);
+
return res;
}
--
1.9.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
