Thread: Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]
Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]
From
Christof Petig
Date:
Hello,
well at first I could not believe what I was seeing ...
Look at the following code (ecpg/lib/execute.c):
const char *locale=setlocale(LC_NUMERIC, NULL);
setlocale(LC_NUMERIC, "C");
[....]
setlocale(LC_NUMERIC, locale);
Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
malloced memory, and frees this pointer on subsequent calls to
setlocale. This is standard conformant and has good reasons. But used as
above it is lethal (but not lethal enough to be easily recognized). So
the content locale points to is freed by the second call to setlocale.
The remedy is easy (given that _no other_ call to setlocale happens
inbetween ...)
const char *locale=setlocale(LC_NUMERIC, "C");
[...]
setlocale(LC_NUMERIC, locale);
So I would kindly ask you to take a second look at every invokation of
setlocale. And to apply the following patch.
Yours
Christof
Attachment
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
> Hello,
>
> well at first I could not believe what I was seeing ...
>
> Look at the following code (ecpg/lib/execute.c):
>
> const char *locale=setlocale(LC_NUMERIC, NULL);
> setlocale(LC_NUMERIC, "C");
> [....]
> setlocale(LC_NUMERIC, locale);
>
>
> Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
> malloced memory, and frees this pointer on subsequent calls to
> setlocale. This is standard conformant and has good reasons. But used as
> above it is lethal (but not lethal enough to be easily recognized). So
> the content locale points to is freed by the second call to setlocale.
>
> The remedy is easy (given that _no other_ call to setlocale happens
> inbetween ...)
>
> const char *locale=setlocale(LC_NUMERIC, "C");
> [...]
> setlocale(LC_NUMERIC, locale);
>
>
> So I would kindly ask you to take a second look at every invokation of
> setlocale. And to apply the following patch.
>
> Yours
> Christof
>
[ application/x-gzip is not supported, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
>> Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
>> malloced memory, and frees this pointer on subsequent calls to
>> setlocale.
>> So I would kindly ask you to take a second look at every invokation of
>> setlocale.
I looked around, and am worried about the behavior of PGLC_current()
in src/backend/utils/adt/pg_locale.c. It doesn't change locale but
does retrieve several successive setlocale() results. Does that work
in glibc?
regards, tom lane
Tom Lane wrote:
> >> Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
> >> malloced memory, and frees this pointer on subsequent calls to
> >> setlocale.
> >> So I would kindly ask you to take a second look at every invokation of
> >> setlocale.
>
> I looked around, and am worried about the behavior of PGLC_current()
> in src/backend/utils/adt/pg_locale.c. It doesn't change locale but
> does retrieve several successive setlocale() results. Does that work
> in glibc?
Well actually I did not check glibc's source code. But I tried to run my
program with efence and it aborted in execute.c
[ locale=setlocale(LC_NUMERIC,NULL);
setlocale(LC_NUMERIC,"C");
...
setlocale(LC_NUMERIC,locale); // access to already freed memory
(locale)
]
So my best guess is that setlocale
- uses a malloced memory for return (which copes best with variable length
strings)
- frees this on a subsequent calls and allocates a new one.
Yes, I'm worried about PGLC_current(), too.
IMHO we should definitely copy the result to a malloced area.
Does the current solution work with static storage (old libcs?)? The last
call would overwrite the first result, wouldn't it?
Christof
On Thu, Sep 27, 2001 at 09:26:12AM +0200, Christof Petig wrote:
> Tom Lane wrote:
>
> > >> Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
> > >> malloced memory, and frees this pointer on subsequent calls to
> > >> setlocale.
> > >> So I would kindly ask you to take a second look at every invokation of
> > >> setlocale.
> >
> > I looked around, and am worried about the behavior of PGLC_current()
> > in src/backend/utils/adt/pg_locale.c. It doesn't change locale but
> > does retrieve several successive setlocale() results. Does that work
> > in glibc?
>
> Well actually I did not check glibc's source code. But I tried to run my
> program with efence and it aborted in execute.c
I see locale/setlocale.c in glibc (I'm very like that PG hasn't same
coding style as glibc developers:-). You are right with strdup()/free()
in the setlocale().
>
> [ locale=setlocale(LC_NUMERIC,NULL);
> setlocale(LC_NUMERIC,"C");
> ...
> setlocale(LC_NUMERIC,locale); // access to already freed memory
> (locale)
> ]
>
> So my best guess is that setlocale
> - uses a malloced memory for return (which copes best with variable length
> strings)
> - frees this on a subsequent calls and allocates a new one.
>
> Yes, I'm worried about PGLC_current(), too.
For example to_char() calls PGLC_localeconv(). In the PGLC_localeconv()
is used:
PGLC_current(&lc);
setlocale(LC_ALL, "");
PGLC_setlocale(&lc); /* <-- access to free memory ? */
I see now it in detail and Christof probably found really pretty bug.
Some users already notice something like:
test=# select to_char(45123.4, 'L99G999D9');
NOTICE: pg_setlocale(): 'LC_MONETARY=pŠ-@č' cannot be honored.
to_char
-------------
Kč 45 123,4
(1 row)
(I use Czech locales)
We don't see this bug often, because PGLC_localeconv() result is cached
and pg_setlocale is called only once.
It must be fixed for 7.2. May be allocate it in PG, because we need
keep data in PG_LocaleCategories independent on glibc's strdup/free.
May be:
PGLC_current(PG_LocaleCategories * lc)
{
lc->lang = getenv("LANG");
PGLC_free_caltegories(lc);
lc->lc_ctype = pstrdup(setlocale(LC_CTYPE, NULL));
... etc.
}
void
PGLC_free_caltegories(PG_LocaleCategories * lc)
{
if (lc->lc_ctype)
pfree(lc->lc_ctype);
...etc.
}
Comments? I right now work on patch for this.
--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/
C, PostgreSQL, PHP, WWW, http://docs.linux.cz, http://mape.jcu.cz
On Thu, Sep 27, 2001 at 12:08:29AM -0400, Tom Lane wrote:
> >> Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
> >> malloced memory, and frees this pointer on subsequent calls to
> >> setlocale.
> >> So I would kindly ask you to take a second look at every invokation of
> >> setlocale.
>
> I looked around, and am worried about the behavior of PGLC_current()
> in src/backend/utils/adt/pg_locale.c. It doesn't change locale but
> does retrieve several successive setlocale() results. Does that work
> in glibc?
The patch is attached. Now it's independent on glibc's game of setlocale()
results and free/strdup. It works for me...
Thanks to Christof!
Karel
--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/
C, PostgreSQL, PHP, WWW, http://docs.linux.cz, http://mape.jcu.cz
Attachment
Re: Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]
From
Michael Meskes
Date:
On Mon, Sep 24, 2001 at 09:18:42AM +0200, Christof Petig wrote:
> well at first I could not believe what I was seeing ...
:-)
> Look at the following code (ecpg/lib/execute.c):
>
> const char *locale=setlocale(LC_NUMERIC, NULL);
> setlocale(LC_NUMERIC, "C");
> [....]
> setlocale(LC_NUMERIC, locale);
>
>
> Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
> malloced memory, and frees this pointer on subsequent calls to
Doesn't look that way on my system. The following programs simply dumps core
in free().
#include <locale.h>
#include <stdio.h>
main()
{
const char *locale=setlocale(LC_NUMERIC, NULL);
printf("%c\n", locale);
free(locale);
}
> setlocale. This is standard conformant and has good reasons. But used as
You're partially right. Standard says "This string may be allocated in
static storage." So, yes, with your patch we are on the safe side. I just
committed the changes.
Michael
--
Michael Meskes
Michael@Fam-Meskes.De
Go SF 49ers! Go Rhein Fire!
Use Debian GNU/Linux! Use PostgreSQL!
Re: Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]
From
Karel Zak
Date:
On Tue, Sep 25, 2001 at 08:15:06PM +0200, Michael Meskes wrote:
> >
> > Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
> > malloced memory, and frees this pointer on subsequent calls to
>
> Doesn't look that way on my system. The following programs simply dumps core
> in free().
>
> #include <locale.h>
> #include <stdio.h>
>
> main()
> {
> const char *locale=setlocale(LC_NUMERIC, NULL);
>
> printf("%c\n", locale);
> free(locale);
> }
Because you bad use setlocale().
The setlocale(LC_NUMERIC, NULL) returns actual LC_NUMERIC setting, but
your program hasn't some setting, because you don't call:
setlocale(LC_NUMERIC, "") or setlocale(LC_NUMERIC, "some_locales")
before setlocale(LC_NUMERIC, NULL), try this program:
#include <stdio.h>
#include <locale.h>
#include <stdlib.h>
int
main()
{
char *locale;
/* create array with locales names */
setlocale(LC_NUMERIC, "");
/* returns data from actual setting */
locale = setlocale(LC_NUMERIC, NULL);
printf("%s\n", locale);
free((void *) locale);
exit(1);
}
and don't forget set LC_ALL before program runnig. With default locales "C"
it is same as with NULL.
Previous code:
$ export LC_ALL="cs_CZ"
$ ./loc
cs_CZ
$ export LC_ALL="C"
$ ./loc
C
Segmentation fault <-- in free()
.... and see locale/setlocale.c in glibc sources :-)
Karel
--
Karel Zak <zakkr@zf.jcu.cz>
http://home.zf.jcu.cz/~zakkr/
C, PostgreSQL, PHP, WWW, http://docs.linux.cz, http://mape.jcu.cz
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
> On Thu, Sep 27, 2001 at 12:08:29AM -0400, Tom Lane wrote:
> > >> Well at least on glibc-2.2 it seems that setlocale retuns a pointer to
> > >> malloced memory, and frees this pointer on subsequent calls to
> > >> setlocale.
> > >> So I would kindly ask you to take a second look at every invokation of
> > >> setlocale.
> >
> > I looked around, and am worried about the behavior of PGLC_current()
> > in src/backend/utils/adt/pg_locale.c. It doesn't change locale but
> > does retrieve several successive setlocale() results. Does that work
> > in glibc?
>
> The patch is attached. Now it's independent on glibc's game of setlocale()
> results and free/strdup. It works for me...
>
> Thanks to Christof!
>
> Karel
>
> --
> Karel Zak <zakkr@zf.jcu.cz>
> http://home.zf.jcu.cz/~zakkr/
>
> C, PostgreSQL, PHP, WWW, http://docs.linux.cz, http://mape.jcu.cz
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Re: Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]
From
Tom Lane
Date:
Michael Meskes <meskes@postgresql.org> writes:
> You're partially right. Standard says "This string may be allocated in
> static storage." So, yes, with your patch we are on the safe side. I just
> committed the changes.
This patch wasn't right: setlocale(LC_NUMERIC, "C") returns a string
corresponding to the *new* locale setting, not the old one. Therefore,
while the patched code failed to dump core, it also failed to restore
the previous locale setting as intended. I have committed an updated
version.
regards, tom lane