Thread: BUG #13762: server will crash after superuser alter function and set client_encoding

The following bug has been logged on the website:

Bug reference:      13762
Logged by:          funnyxj
Email address:      funnyxj@qq.com
PostgreSQL version: 9.4.5
Operating system:   linux
Description:

When superuser execute the following statements, the server will crash, it
may be a bug.
statements:

1. alter function koi8r_to_utf8(integer, integer, cstring, internal,
integer) set work_mem = default ;
2. set client_encoding = 'koi8';
3. select 1;

then the error log is:
server closed the connection unexpectedly
    This probably means the server terminated abnormally
    before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>

On Mon, Nov 9, 2015 at 9:41 PM,  <funnyxj@qq.com> wrote:
> The following bug has been logged on the website:
>
> Bug reference:      13762
> Logged by:          funnyxj
> Email address:      funnyxj@qq.com
> PostgreSQL version: 9.4.5
> Operating system:   linux
> Description:
>
> When superuser execute the following statements, the server will crash, it
> may be a bug.
> statements:
>
> 1. alter function koi8r_to_utf8(integer, integer, cstring, internal,
> integer) set work_mem = default ;
> 2. set client_encoding = 'koi8';
> 3. select 1;

I am not able to reproduce this issue.

[kommih@localhost bin]$ ./psql postgres
psql (9.4.5)
Type "help" for help.

postgres=# alter function koi8r_to_utf8(integer, integer, cstring, internal,
integer) set work_mem = default ;
ALTER FUNCTION
postgres=# set client_encoding = 'koi8';
SET
postgres=# select 1;
 ?column?
----------
        1
(1 row)


Regards,
Hari Babu
Fujitsu Australia

funnyxj@qq.com writes:
> When superuser execute the following statements, the server will crash, it
> may be a bug.

> 1. alter function koi8r_to_utf8(integer, integer, cstring, internal,
> integer) set work_mem = default ;

That would be in the category of "don't do that".  Encoding conversion
functions are invoked using special code paths that don't support every
possible option for functions, so I'm not too surprised that this breaks
things (and I see no reason to expend effort to try to make it not be
broken).

FWIW, I can't reproduce a crash with the described case either, so
I think there's probably some additional condition required.  But
it doesn't really matter.  Mucking with the properties of built-in
functions isn't considered a supported thing to do.

            regards, tom lane

On Tue, Nov 10, 2015 at 9:51 AM, 笑见江湖 <funnyxj@qq.com> wrote:
> Although its is in the category of "don't do that",  but server crashed is
> dangerous. If I alter a function to "set work_mem = '4MB'", it should be no
> problem and reasonable.

ALTERing a system function reserved for encoding is not that much a
good idea, following Tom's point.

> Sorry, the correct statements which make server crash is:
>
> 1. alter function koi8r_to_utf8(integer, integer, cstring, internal,
> integer) set work_mem = '4MB';
> 2. set client_encoding = 'koi8';
> 3. select 1;

This indeed crashes, and that's not limited to this particular
function, try for example iso8859_to_utf8 with ISO_8859_6 as
client_encoding on even a different parameter for the same result.
FWIW, here is a backtrace with assertions enabled:
    frame #2: 0x000000010b3a9949
postgres`ExceptionalCondition(conditionName=0x000000010b4a9b13,
errorType=0x000000010b417495, fileName=0x000000010b4cd1cf,
lineNumber=1127) + 137 at assert.c:54
    frame #3: 0x000000010b383709
postgres`SearchCatCache(cache=0x00007fa36985e5f8, v1=12262, v2=0,
v3=0, v4=0) + 89 at catcache.c:1127
    frame #4: 0x000000010b39dc0c postgres`SearchSysCache(cacheId=42,
key1=12262, key2=0, key3=0, key4=0) + 172 at syscache.c:981
    frame #5: 0x000000010b3b4333
postgres`fmgr_info_cxt_security(functionId=12262,
finfo=0x00007fa369872a88, mcxt=0x00007fa369404610,
ignore_security='\x01') + 227 at fmgr.c:215
    frame #6: 0x000000010b3b92a3
postgres`fmgr_security_definer(fcinfo=0x00007fff54e7c8a8) + 115 at
fmgr.c:905
    frame #7: 0x000000010b3b664c
postgres`FunctionCall5Coll(flinfo=0x00007fa369872e60, collation=0,
arg1=26, arg2=6, arg3=140339826604600, arg4=140339826606696, arg5=9) +
220 at fmgr.c:1404
    frame #8: 0x000000010b3c810e
postgres`perform_default_encoding_conversion(src=0x00007fa369833a38,
len=9, is_client_to_server='\x01') + 382 at mbutils.c:715
    frame #9: 0x000000010b3c7f5b
postgres`pg_any_to_server(s=0x00007fa369833a38, len=9, encoding=26) +
459 at mbutils.c:619
    frame #10: 0x000000010b3c7d85
postgres`pg_client_to_server(s=0x00007fa369833a38, len=9) + 37 at
mbutils.c:558

frame #3: 0x000000010b383709
postgres`SearchCatCache(cache=0x00007fa36985e5f8, v1=12262, v2=0,
v3=0, v4=0) + 89 at catcache.c:1127
   1124        HeapTuple    ntp;
   1125
   1126        /* Make sure we're in an xact, even if this ends up
being a cache hit */
-> 1127        Assert(IsTransactionState());

This is basically trying to do a relcache lookup without being in a
transaction block, so well... That's really in the category of don't
do that. Encoding conversions are not aimed at happening within a
transaction context.
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:
> On Tue, Nov 10, 2015 at 9:51 AM, 笑见江湖 <funnyxj@qq.com> wrote:
>> Sorry, the correct statements which make server crash is:
>>
>> 1. alter function koi8r_to_utf8(integer, integer, cstring, internal,
>> integer) set work_mem = '4MB';
>> 2. set client_encoding = 'koi8';
>> 3. select 1;

> This indeed crashes, and that's not limited to this particular
> function, try for example iso8859_to_utf8 with ISO_8859_6 as
> client_encoding on even a different parameter for the same result.
> FWIW, here is a backtrace with assertions enabled:

Yeah... if you trace through this, you'll notice that (1) calling a
function having non-null proconfig requires fetching its pg_proc entry,
and (2) fetching anything from a catalog requires being inside a live
transaction, and (3) the client/server encoding translation functions have
to be invokable outside any transaction.  So as I said, this is not
going to work and we are not very interested in making it work.

            regards, tom lane