From 0c4457de8b8a6aa715bdfe94ecfa8ee3d48e2827 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 28 Feb 2016 23:39:20 -0500 Subject: [PATCH] Avoid multiple free_struct_lconv() calls on same data. A failure partway through PGLC_localeconv() led to a situation where the next call would call free_struct_lconv() a second time, leading to free() on already-freed strings, typically leading to a core dump. Add a flag to remember whether we need to do that. Per report from Thom Brown. His example case only provokes the failure as far back as 9.4, but nonetheless this code is obviously broken, so back-patch to all supported branches. --- src/backend/utils/adt/pg_locale.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index a0b06953372..aa8144d5627 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -363,9 +363,6 @@ assign_locale_messages(const char *newval, void *extra) static void free_struct_lconv(struct lconv * s) { - if (s == NULL) - return; - if (s->currency_symbol) free(s->currency_symbol); if (s->decimal_point) @@ -419,6 +416,7 @@ struct lconv * PGLC_localeconv(void) { static struct lconv CurrentLocaleConv; + static bool CurrentLocaleConvAllocated = false; struct lconv *extlconv; char *save_lc_monetary; char *save_lc_numeric; @@ -435,7 +433,12 @@ PGLC_localeconv(void) if (CurrentLocaleConvValid) return &CurrentLocaleConv; - free_struct_lconv(&CurrentLocaleConv); + /* Free any already-allocated storage */ + if (CurrentLocaleConvAllocated) + { + free_struct_lconv(&CurrentLocaleConv); + CurrentLocaleConvAllocated = false; + } /* Save user's values of monetary and numeric locales */ save_lc_monetary = setlocale(LC_MONETARY, NULL); @@ -499,7 +502,9 @@ PGLC_localeconv(void) /* * Must copy all values since restoring internal settings may overwrite - * localeconv()'s results. + * localeconv()'s results. Note that if we were to fail within this + * sequence before reaching "CurrentLocaleConvAllocated = true", we could + * leak some memory --- but not much, so it's not worth agonizing over. */ CurrentLocaleConv = *extlconv; CurrentLocaleConv.decimal_point = decimal_point; @@ -512,6 +517,7 @@ PGLC_localeconv(void) CurrentLocaleConv.mon_thousands_sep = db_encoding_strdup(encoding, extlconv->mon_thousands_sep); CurrentLocaleConv.negative_sign = db_encoding_strdup(encoding, extlconv->negative_sign); CurrentLocaleConv.positive_sign = db_encoding_strdup(encoding, extlconv->positive_sign); + CurrentLocaleConvAllocated = true; /* Try to restore internal settings */ if (save_lc_monetary) -- 2.39.5