Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#4174 closed defect (fixed)

segfault when using spgist

Reported by: tobwen Owned by: pramsey
Priority: medium Milestone: PostGIS PostgreSQL
Component: postgis Version: master
Keywords: Cc:

Description

I'm on Debian Stretch, AMD64 with PostgreSQL 11b3 and PostGIS 2.5 from PostgreSQL's repository.

On this (non optimized and ugly) query, I'm getting a segfault after a second:

SELECT
    b.idhu, layer, max(dba)
FROM
    (SELECT b."IDHU" AS idhu, first(geom) AS geom FROM do_buildings b JOIN do_data d ON b."IDHU"::text = d."IDHU"::text GROUP BY 1 LIMIT 100) b
CROSS JOIN unnest('{str_isof_den}'::text[]) AS lyr
CROSS JOIN LATERAL (
     SELECT idhu, layer, dba
     FROM   do_laerm l
     WHERE  layer = lyr AND ST_Intersects(l.geom, b.geom)
     LIMIT 1
) AS target
GROUP by
   1, 2;

I've got these two spgist indexes on the geoms (it works with normal gist):

CREATE INDEX idx_do_laerm ON do_laerm USING spgist(geom);
CREATE INDEX idx_do_buildings ON do_buildings USING spgist(geom);

The dbg trace is attached to this report.

Attachments (1)

3a7532d1970d.txt (4.7 KB ) - added by TobWen 6 years ago.

Download all attachments as: .zip

Change History (4)

by TobWen, 6 years ago

Attachment: 3a7532d1970d.txt added

comment:1 by TobWen, 6 years ago

RhodiumToad on #postgresql analyzed this. It might be a problem in core!

<RhodiumToad> I think this can only be triggered by a LATERAL subquery with a LIMIT that does an spgist index scan using an opclass that stores traversal values
<RhodiumToad> which I don't think the builtin opclasses do, but postgis does
<FunkyBob> RhodiumToad: so... it's moderately obscure to trigger?
<RhodiumToad> spgrescan starts out by doing MemoryContextReset(so->traversalCxt);
<RhodiumToad> but then later calls resetSpGistScanOpaque(so);
<RhodiumToad> and that calls freeScanStack
<RhodiumToad> and that calls freeScanStackEntry
<RhodiumToad> and that does  if (stackEntry->traversalValue) pfree(stackEntry->traversalValue);
<RhodiumToad> but stackEntry->traversalValue is supposed to be allocated inside so->traversalCxt
<RhodiumToad> so it can only break if you do a rescan while there's already a scan in progress that has entries stacked
<RhodiumToad> so it may not just be that it's a lateral (... limit) query, it may also depend on the particular point at which the limit is hit
<RhodiumToad> I'm going to post about this to hackers
Last edited 6 years ago by TobWen (previous) (diff)

comment:2 by robe, 6 years ago

Milestone: PostGIS 2.5.0PostGIS PostgreSQL
Resolution: fixed
Status: newclosed

Fixed upstream https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=e331d6712f0224160d2699591704ddcc3ef2d67b

Summary

    Repair double-free in SP-GIST rescan (bug #15378) (details)

Commit e331d6712f0224160d2699591704ddcc3ef2d67b by rhodiumtoad

Repair double-free in SP-GIST rescan (bug #15378)
spgrescan would first reset traversalCxt, and then traverse a 
potentially non-empty stack containing pointers to traversalValues which
had been allocated in those contexts, freeing them a second time. This
bug originates in commit ccd6eb49a where traversalValue was introduced.
Repair by traversing the stack before the context reset; this isn't 
ideal, since it means doing retail pfree in a context that's about to be
reset, but the freeing of a stack entry is also done in other places in
the code during the scan so it's not worth trying to refactor it
further. Regression test added.
Backpatch to 9.6 where the problem was introduced.
Per bug #15378; analysis and patch by me, originally from a report on 
IRC by user velix; see also PostGIS ticket #4174; review by Alexander 
Korotkov.
Discussion:
https://postgr.es/m/153663176628.23136.11901365223750051490@wrigleys.postgresql.org

The file was modified	src/test/regress/expected/spgist.out
The file was modified	src/test/regress/sql/spgist.sql
The file was modified	src/backend/access/spgist/spgscan.c

comment:3 by robe, 3 years ago

Reporter: changed from TobWen to tobwen
Note: See TracTickets for help on using tickets.