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
Mon, 24 Sep 2001 09:18:42 +0200
Christof Petig <christof@petig-baender.de>
Problem with setlocale (found in libecpg) [accessing a memory
	location after freeing it]
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!

Tue, 25 Sep 2001 20:15:06 +0200
Michael Meskes <meskes@postgresql.org>
Re: Problem with setlocale (found in libecpg) [accessing a memory
	location after freeing it]

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

Wed, 26 Sep 2001 16:22:06 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Problem with setlocale (found in libecpg) [accessing a
>> 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

Thu, 27 Sep 2001 00:08:29 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Problem with setlocale (found in libecpg) [accessing a 
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



Thu, 27 Sep 2001 09:26:12 +0200
Christof Petig <christof@petig-baender.de>
Re: [PATCHES] Problem with setlocale (found in libecpg) [accessing a
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

Thu, 27 Sep 2001 10:49:24 +0200
Karel Zak <zakkr@zf.jcu.cz>
Re: [PATCHES] Problem with setlocale (found in libecpg) [accessing a
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
Thu, 27 Sep 2001 12:11:15 +0200
Karel Zak <zakkr@zf.jcu.cz>
pg_locale (Was: Re: Problem with setlocale (found in libecpg)...)
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.]



Thu, 27 Sep 2001 12:17:31 +0200
Christof Petig <christof@petig-baender.de>
Re: [PATCHES] Problem with setlocale (found in libecpg) [accessing a
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

Thu, 27 Sep 2001 17:26:56 +0200
Karel Zak <zakkr@zf.jcu.cz>
Re: Problem with setlocale (found in libecpg) [accessing a memory
	location after freeing it]

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

Fri, 28 Sep 2001 15:37:41 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: pg_locale (Was: Re: Problem with setlocale (found in libecpg)...)
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

Sat, 29 Sep 2001 16:15:01 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: Problem with setlocale (found in libecpg) [accessing a memory
	location after freeing it]