Thread: hashtable crash (was Re: [PATCHES] Post-mortem: final 2PC patch)

hashtable crash (was Re: [PATCHES] Post-mortem: final 2PC patch)

From
Tom Lane
Date:
Oh-ho, I see it:

(gdb) bt
#0  0x000000004489fba4 in memcpy () from /usr/lib/libc.so.34.2
#1  0x0000000000326f9c in hash_search (hashp=0xa6e030, keyPtr=0xa5ff90,   action=HASH_ENTER, foundPtr=0x0) at
dynahash.c:653
#2  0x00000000003434f0 in pg_tzset (name=0xa5ff90 "PST8PDT") at pgtz.c:1039
#3  0x00000000001fbcf0 in assign_timezone (value=0xa5ff90 "PST8PDT",   doit=1 '\001', source=PGC_S_CLIENT) at
variable.c:351
...
(gdb) f 1
#1  0x0000000000326f9c in hash_search (hashp=0xa6e030, keyPtr=0xa5ff90,   action=HASH_ENTER, foundPtr=0x0) at
dynahash.c:653
653                             memcpy(ELEMENTKEY(currBucket), keyPtr, hctl->keysize);
(gdb) p *hctl
$1 = {dsize = 256, ssize = 256, sshift = 8, max_bucket = 31, high_mask = 63, low_mask = 31, ffactor = 1, nentries = 1,
nsegs= 1, keysize = 255, entrysize = 11064, max_dsize = -1, freeList = 0xaca758}
 
(gdb) p keyPtr
$2 = (const void *) 0xa5ff90
(gdb) x/255c 0xa5ff90
0xa5ff90:       80 'P'  83 'S'  84 'T'  56 '8'  80 'P'  68 'D'  84 'T'  0 '\0'
0xa5ff98:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffa0:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffa8:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffb0:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffb8:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffc0:       80 'P'  111 'o' 115 's' 116 't' 103 'g' 114 'r' 101 'e' 115 's'
0xa5ffc8:       44 ','  32 ' '  77 'M'  68 'D'  89 'Y'  0 '\0'  0 '\0'  0 '\0'
0xa5ffd0:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffd8:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffe0:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5ffe8:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa5fff0:       36 '$'  108 'l' 105 'i' 98 'b'  100 'd' 105 'i' 114 'r' 0 '\0'
0xa5fff8:       0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'  0 '\0'
0xa60000:       Cannot access memory at address 0xa60000
(gdb) 

dynahash.c thinks it should always copy 255 bytes of key, since that's
what it was told the key size was ... but in this case the supplied
search key has been allocated very close to the end of the process's
memory, and there are not 255 bytes before the end of memory.

I'm surprised we have not seen cases like this reported before, because
the risk has been there since at least PG 7.4 (or whenever we started
allowing un-padded C strings as dynahash keys).  Will fix.
        regards, tom lane


Re: hashtable crash (was Re: [PATCHES] Post-mortem: final 2PC patch)

From
Stefan Kaltenbrunner
Date:
Tom Lane wrote:

> dynahash.c thinks it should always copy 255 bytes of key, since that's
> what it was told the key size was ... but in this case the supplied
> search key has been allocated very close to the end of the process's
> memory, and there are not 255 bytes before the end of memory.

aaah - this description rings a bell ...

OpenBSD has some very useful features for configuration of malloc() -
and on this particular box it has:

G       ``Guard''.  Enable guard pages and chunk randomization.  Each            page size or larger allocation is
followedby a guard page that            will cause a segmentation fault upon any access.  Smaller than            page
sizechunks are returned in a random order.
 


and indeed - enabling "G" on another (x86) OpenBSD box of mine causes
make check to die there too ....


Stefan


Re: hashtable crash (was Re: [PATCHES] Post-mortem: final 2PC patch)

From
Tom Lane
Date:
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> Tom Lane wrote:
>> dynahash.c thinks it should always copy 255 bytes of key, since that's
>> what it was told the key size was ... but in this case the supplied
>> search key has been allocated very close to the end of the process's
>> memory, and there are not 255 bytes before the end of memory.

> aaah - this description rings a bell ...

> OpenBSD has some very useful features for configuration of malloc() -
> and on this particular box it has:

> G       ``Guard''.  Enable guard pages and chunk randomization.  Each
>              page size or larger allocation is followed by a guard page that
>              will cause a segmentation fault upon any access.  Smaller than
>              page size chunks are returned in a random order.

> and indeed - enabling "G" on another (x86) OpenBSD box of mine causes
> make check to die there too ....

Cool.  Once I get this bug fixed, the people running openbsd build farm
machines probably should turn that on as standard practice ... we've
found bugs of this ilk several times before, and I would not be
surprised if there are more.

The palloc mechanism probably does quite a lot to mask this type of
error, since it aggregates small chunk requests into large malloc
chunks.  If you read past the end of a palloc'd chunk it's quite
unlikely that you'll see a problem.  I wonder if it is worth having
a debug-build option that defeats that somehow ...
        regards, tom lane


Re: hashtable crash (was Re: [PATCHES] Post-mortem: final

From
Andrew Dunstan
Date:

Tom Lane wrote:

>Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>  
>
>
>
>  
>
>>OpenBSD has some very useful features for configuration of malloc() -
>>and on this particular box it has:
>>    
>>
>
>  
>
>>G       ``Guard''.  Enable guard pages and chunk randomization.  Each
>>             page size or larger allocation is followed by a guard page that
>>             will cause a segmentation fault upon any access.  Smaller than
>>             page size chunks are returned in a random order.
>>    
>>
>
>  
>
>>and indeed - enabling "G" on another (x86) OpenBSD box of mine causes
>>make check to die there too ....
>>    
>>
>
>Cool.  Once I get this bug fixed, the people running openbsd build farm
>machines probably should turn that on as standard practice ... we've
>found bugs of this ilk several times before, and I would not be
>surprised if there are more.
>
>
>  
>

Stefan currently has the only openbsd members. However. I will probably 
add something to the buildfarm config file to turn on some of those 
options on openbsd. Can you please look at 

http://www.openbsd.org/cgi-bin/man.cgi?query=malloc.conf&apropos=0&sektion=0&manpath=OpenBSD+Current&arch=i386&format=html

and tell us which ones you would like turned on? Stefan suggests A F and 
G might be useful.

cheers

andrew


Re: hashtable crash (was Re: [PATCHES] Post-mortem: final 2PC patch)

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> Can you please look at 
>
http://www.openbsd.org/cgi-bin/man.cgi?query=malloc.conf&apropos=0&sektion=0&manpath=OpenBSD+Current&arch=i386&format=html

> and tell us which ones you would like turned on? Stefan suggests A F and 
> G might be useful.

Not "A" please --- that's a pretty significant change in the behavior
of malloc and would break code that expects to be able to tolerate
out-of-memory conditions (which we do have, see palloc).  F and G are
good, also J, maybe P though I doubt that one's very significant.
        regards, tom lane