Horton
Menu

Thread

  1. Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]

    Christof Petig <christof@petig-baender.de> — 2001-09-24T07:18:42Z

    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

  2. Re: Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]

    Michael Meskes <meskes@postgresql.org> — 2001-09-25T18:15:06Z

    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!

  3. Re: Problem with setlocale (found in libecpg) [accessing a

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-09-26T20:22:06Z

    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

  4. Re: Problem with setlocale (found in libecpg) [accessing a

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-09-27T04:08:29Z

    >> 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

  5. Re: [PATCHES] Problem with setlocale (found in libecpg) [accessing a

    Christof Petig <christof@petig-baender.de> — 2001-09-27T07:26:12Z

    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

  6. Re: [PATCHES] Problem with setlocale (found in libecpg) [accessing a

    Karel Zak <zakkr@zf.jcu.cz> — 2001-09-27T08:49:24Z

    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

  7. pg_locale (Was: Re: Problem with setlocale (found in libecpg)...)

    Karel Zak <zakkr@zf.jcu.cz> — 2001-09-27T10:11:15Z

    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

  8. Re: [PATCHES] Problem with setlocale (found in libecpg) [accessing a

    Christof Petig <christof@petig-baender.de> — 2001-09-27T10:17:31Z

    Michael Meskes wrote:

> On Thu, Sep 27, 2001 at 12:08:29AM -0400, Tom Lane wrote:
> > 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?
>
> I haven't experienced any problem so far, but then I wasn't able to
> reproduce Christof's either on my glibc2.2 system.

You have to link with efence to see it (see below). (BTW the bug is in
libecpg)

Otherwise the bug is hidden (setting an illegal locale simply does not do
anything if we ignore it's return value (setlocale returns NULL on
error)). Perhaps outputting a notice to the debug stream if setlocale
fails is a good choice (I don't like to raise a SQL error).

Christof

[More detailed: if the former value is freed, the pointer still points to
a valid memory region (without efence), further processing inside ecpg
will reuse that region for just another string (an input variable's value
in SQL notation).
So setting locale '0' or 'ISO' or 'some string' silently fails.]

  9. Re: Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]

    Karel Zak <zakkr@zf.jcu.cz> — 2001-09-27T15:26:56Z

    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

  10. Re: pg_locale (Was: Re: Problem with setlocale (found in libecpg)...)

    Bruce Momjian <pgman@candle.pha.pa.us> — 2001-09-28T19:37:41Z

    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

  11. Re: Problem with setlocale (found in libecpg) [accessing a memory location after freeing it]

    Tom Lane <tgl@sss.pgh.pa.us> — 2001-09-29T20:15:01Z

    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