Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 907e4dd

Browse files
committed
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.
1 parent 26fdff1 commit 907e4dd

File tree

1 file changed

+11
-5
lines changed

1 file changed

+11
-5
lines changed

src/backend/utils/adt/pg_locale.c

+11-5
Original file line numberDiff line numberDiff line change
@@ -387,9 +387,6 @@ assign_locale_messages(const char *newval, void *extra)
387387
static void
388388
free_struct_lconv(struct lconv * s)
389389
{
390-
if (s == NULL)
391-
return;
392-
393390
if (s->currency_symbol)
394391
free(s->currency_symbol);
395392
if (s->decimal_point)
@@ -441,6 +438,7 @@ struct lconv *
441438
PGLC_localeconv(void)
442439
{
443440
static struct lconv CurrentLocaleConv;
441+
static bool CurrentLocaleConvAllocated = false;
444442
struct lconv *extlconv;
445443
char *save_lc_monetary;
446444
char *save_lc_numeric;
@@ -457,7 +455,12 @@ PGLC_localeconv(void)
457455
if (CurrentLocaleConvValid)
458456
return &CurrentLocaleConv;
459457

460-
free_struct_lconv(&CurrentLocaleConv);
458+
/* Free any already-allocated storage */
459+
if (CurrentLocaleConvAllocated)
460+
{
461+
free_struct_lconv(&CurrentLocaleConv);
462+
CurrentLocaleConvAllocated = false;
463+
}
461464

462465
/* Save user's values of monetary and numeric locales */
463466
save_lc_monetary = setlocale(LC_MONETARY, NULL);
@@ -521,7 +524,9 @@ PGLC_localeconv(void)
521524

522525
/*
523526
* Must copy all values since restoring internal settings may overwrite
524-
* localeconv()'s results.
527+
* localeconv()'s results. Note that if we were to fail within this
528+
* sequence before reaching "CurrentLocaleConvAllocated = true", we could
529+
* leak some memory --- but not much, so it's not worth agonizing over.
525530
*/
526531
CurrentLocaleConv = *extlconv;
527532
CurrentLocaleConv.decimal_point = decimal_point;
@@ -534,6 +539,7 @@ PGLC_localeconv(void)
534539
CurrentLocaleConv.mon_thousands_sep = db_encoding_strdup(encoding, extlconv->mon_thousands_sep);
535540
CurrentLocaleConv.negative_sign = db_encoding_strdup(encoding, extlconv->negative_sign);
536541
CurrentLocaleConv.positive_sign = db_encoding_strdup(encoding, extlconv->positive_sign);
542+
CurrentLocaleConvAllocated = true;
537543

538544
/* Try to restore internal settings */
539545
if (save_lc_monetary)

0 commit comments

Comments
 (0)