Horton

Thread

Commits

  1. Fix crash in contrib/ltree's lca() function for empty input array.

  1. function lca('{}'::ltree[]) caused DB Instance crash

    李海龙 <hailong.li@qunar.com> — 2018-07-13T10:09:20Z

    
HI,Oleg && pgsql-hackers

Plese help me to check this is a bug of ltree?


thxs!




lhl@localhost:~$ cat /etc/issue
Ubuntu 14.04.5 LTS \n \l

lhl@localhost:~$ uname -av
Linux localhost 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux


lhl@localhost:~$ psql mydb
psql (10.3)
Type "help" for help.

mydb=# select version();
                                                version
-------------------------------------------------------------------------------------------------------
 PostgreSQL 10.3 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4, 64-bit
(1 row)

mydb=# \dx ltree
                        List of installed extensions
 Name  | Version | Schema |                   Description
-------+---------+--------+-------------------------------------------------
 ltree | 1.1     | public | data type for hierarchical tree-like structures
(1 row)

mydb=# select lca('{}'::ltree[]);
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.




in /var/log/syslog

Jul 13 18:02:39 localhost kernel: [25703.907912] postgres[8869]: segfault at 4 ip 00007f6bad8c2ca6 sp 00007fffd8f64560 error 4 in ltree.so[7f6bad8be000+d000]


in log of PostgreSQL Datadir

[lhl mydb psql [local] 2018-07-13 18:02:36.962 CST 8869 5b4878b6.22a5 1 3/2 0]LOG:  statement: SELECT e.extname AS "Name", e.extversion AS "Version", n.nspname AS "Schema", c.description AS "Description"
    FROM pg_catalog.pg_extension e LEFT JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace LEFT JOIN pg_catalog.pg_description c ON c.objoid = e.oid AND c.classoid = 'pg_catalog.pg_extension'::pg_catalog.regclass
    WHERE e.extname OPERATOR(pg_catalog.~) '^(ltree)$'
    ORDER BY 1;
[lhl mydb psql [local] 2018-07-13 18:02:36.972 CST 8869 5b4878b6.22a5 2 3/0 0]LOG:  duration: 10.269 ms
[lhl mydb psql [local] 2018-07-13 18:02:39.313 CST 8869 5b4878b6.22a5 3 3/3 0]LOG:  statement: select lca('{}'::ltree[]);
[    2018-07-13 18:02:39.444 CST 1638 5b481476.666 102  0]LOG:  server process (PID 8869) was terminated by signal 11: Segmentation fault
[    2018-07-13 18:02:39.444 CST 1638 5b481476.666 103  0]DETAIL:  Failed process was running: select lca('{}'::ltree[]);
[    2018-07-13 18:02:39.444 CST 1638 5b481476.666 104  0]LOG:  terminating any other active server processes
[lhl mydb [unknown] [local] 2018-07-13 18:02:39.445 CST 8873 5b4878bf.22a9 1  0]FATAL:  the database system is in recovery mode
[    2018-07-13 18:02:39.446 CST 8635 5b4876a7.21bb 1 1/0 0]WARNING:  terminating connection because of crash of another server process
[    2018-07-13 18:02:39.446 CST 8635 5b4876a7.21bb 2 1/0 0]DETAIL:  The postmaster has commanded this server process to roll back the current transaction and exit, because another server process exited abnormally and possibly corrupted shared memory.
[    2018-07-13 18:02:39.446 CST 8635 5b4876a7.21bb 3 1/0 0]HINT:  In a moment you should be able to reconnect to the database and repeat your command.
[    2018-07-13 18:02:39.447 CST 1638 5b481476.666 105  0]LOG:  archiver process (PID 8636) exited with exit code 1
[    2018-07-13 18:02:39.447 CST 1638 5b481476.666 106  0]LOG:  all server processes terminated; reinitializing
[    2018-07-13 18:02:39.520 CST 8874 5b4878bf.22aa 1  0]LOG:  database system was interrupted; last known up at 2018-07-13 17:53:42 CST
[    2018-07-13 18:02:39.798 CST 8874 5b4878bf.22aa 2  0]LOG:  database system was not properly shut down; automatic recovery in progress
[    2018-07-13 18:02:39.838 CST 8874 5b4878bf.22aa 3  0]LOG:  redo starts at 1/9815F478
[    2018-07-13 18:02:39.838 CST 8874 5b4878bf.22aa 4  0]LOG:  invalid record length at 1/9815F4B0: wanted 24, got 0
[    2018-07-13 18:02:39.839 CST 8874 5b4878bf.22aa 5  0]LOG:  redo done at 1/9815F478
[    2018-07-13 18:02:40.036 CST 1638 5b481476.666 107  0]LOG:  database system is ready to accept connections



--
PostgreSQL DBA hailong.li

  2. Re: function lca('{}'::ltree[]) caused DB Instance crash

    Pierre Ducroquet <p.psql@pinaraf.info> — 2018-07-13T10:48:35Z

    On Friday, July 13, 2018 12:09:20 PM CEST 李海龙 wrote:
> HI,Oleg && pgsql-hackers
> 
> Plese help me to check this is a bug of ltree?
> 

Hi

There is indeed a bug. The _lca function in _ltree_op.c tries to allocate 0 
bytes of memory, doesn't initialize it and dereference it in lca_inner.
The attached basic patch fixes it.

Regards

 Pierre

  3. Re: function lca('{}'::ltree[]) caused DB Instance crash

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-07-13T14:26:56Z

    Pierre Ducroquet <p.psql@pinaraf.info> writes:
> On Friday, July 13, 2018 12:09:20 PM CEST 李海龙 wrote:
>> contrib_regression=# select lca('{}'::ltree[]);
>> server closed the connection unexpectedly

> There is indeed a bug. The _lca function in _ltree_op.c tries to allocate 0 
> bytes of memory, doesn't initialize it and dereference it in lca_inner.
> The attached basic patch fixes it.

I think the defense ought to be in lca_inner not there.

However, I don't understand why this code is returning NULL, rather than
a zero-length ltree, in the case that there's no common prefix.  That
doesn't seem consistent to me.

			regards, tom lane

  4. Re: function lca('{}'::ltree[]) caused DB Instance crash

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-07-13T22:50:33Z

    I wrote:
> However, I don't understand why this code is returning NULL, rather than
> a zero-length ltree, in the case that there's no common prefix.  That
> doesn't seem consistent to me.

After looking more closely, I see that what lca() returns is the longest
common *ancestor* of the input paths, not the longest common *prefix*
... at least, by my understanding of what a prefix is.  If the longest
prefix is empty, then there's no common ancestor, so returning null in
that case isn't so insane after all.  However, the documentation seems
very misleading on this point.  I changed it around along with pushing
the crash fix.

I wonder whether there isn't a need for an lcp() that would return the
common prefix as that's usually understood.

			regards, tom lane