topology: crash when using ST_AddEdgeModFace on face that's part of multiple topogeometries

[rulus]

When using ST_AddEdgeModFace to add a new edge splitting a face that's part of more than 1 topogeometry, postgres crashes with a segfault.

I am running version PostgreSQL 9.4.5 on x86_64-unknown-linux-gnu, compiled by gcc (Debian 5.2.1-23) 5.2.1 20151028, 64-bit POSTGIS="2.2.0 r14208" GEOS="3.5.0-CAPI-1.9.0 r4084" PROJ="Rel. 4.9.2, 08 September 2015" GDAL="GDAL 2.0.1, released 2015/09/15" LIBXML="2.9.3" LIBJSON="0.11.99" TOPOLOGY RASTER

Using gdb I got this backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00007fd5ac1d8358 in cb_updateTopoGeomFaceSplit (topo=0x55c79d7b6ae0, split_face=<optimized out>, new_face1=133, new_face2=-1) at postgis_topology.c:2018
2018	    HeapTuple row = SPI_tuptable->vals[i];
(gdb) bt
#0  0x00007fd5ac1d8358 in cb_updateTopoGeomFaceSplit (topo=0x55c79d7b6ae0, split_face=<optimized out>, new_face1=133, new_face2=-1) at postgis_topology.c:2018
#1  0x00007fd5ac201cfd in lwt_be_updateTopoGeomFaceSplit (topo=0x55c79d7b6b48, topo=0x55c79d7b6b48, new_face2=-1, new_face1=133, split_face=<optimized out>) at lwgeom_topo.c:350
#2  _lwt_AddEdge (topo=topo@entry=0x55c79d7b6b48, start_node=start_node@entry=142, end_node=end_node@entry=208, geom=geom@entry=0x55c79d7b46f0, skipChecks=skipChecks@entry=0, modFace=modFace@entry=1)
    at lwgeom_topo.c:2696
#3  0x00007fd5ac20500b in lwt_AddEdgeModFace (topo=topo@entry=0x55c79d7b6b48, start_node=start_node@entry=142, end_node=end_node@entry=208, geom=geom@entry=0x55c79d7b46f0, skipChecks=skipChecks@entry=0)
    at lwgeom_topo.c:2722
#4  0x00007fd5ac1d9f20 in ST_AddEdgeModFace (fcinfo=0x55c79d7df650) at postgis_topology.c:3207
#5  0x000055c79cd5ee83 in ?? ()
#6  0x000055c79cd64e6d in ExecProject ()
#7  0x000055c79cd77568 in ExecResult ()
#8  0x000055c79cd5dd18 in ExecProcNode ()
#9  0x000055c79cd5af5e in standard_ExecutorRun ()
#10 0x000055c79cd8188f in ?? ()
#11 0x000055c79cd81bac in SPI_execute_plan_with_paramlist ()
#12 0x00007fd5b36094a4 in ?? () from /usr/lib/postgresql/9.4/lib/plpgsql.so
#13 0x00007fd5b360d6c5 in ?? () from /usr/lib/postgresql/9.4/lib/plpgsql.so
#14 0x00007fd5b360faad in ?? () from /usr/lib/postgresql/9.4/lib/plpgsql.so
#15 0x00007fd5b360fcd2 in plpgsql_exec_function () from /usr/lib/postgresql/9.4/lib/plpgsql.so
#16 0x00007fd5b3604d49 in plpgsql_call_handler () from /usr/lib/postgresql/9.4/lib/plpgsql.so
#17 0x000055c79cd5ee83 in ?? ()
#18 0x000055c79cd64e6d in ExecProject ()
#19 0x000055c79cd77568 in ExecResult ()
#20 0x000055c79cd5dd18 in ExecProcNode ()
#21 0x000055c79cd5af5e in standard_ExecutorRun ()
#22 0x000055c79ce55247 in ?? ()
#23 0x000055c79ce56888 in PortalRun ()
#24 0x000055c79ce534e3 in PostgresMain ()
#25 0x000055c79cc1c773 in ?? ()
#26 0x000055c79cdfd50a in PostmasterMain ()
#27 0x000055c79cc1d90c in main ()

Leading me to the cb_updateTopoGeomFaceSplit function in topology/postgis_topology.c (I used the 2.2.0 release from the git tag).

The problem seems to occur when the first SELECT query yields > 1 result, in which case ntopogeoms > 1 and the following loop is executed multiple times.

Inside the loop the global SPI_tuptable variable is used to access the results from the SELECT query. However, inside the loop other INSERT queries are issued overwriting this SPI_tuptable variable causing the crash in the second iteration.

I tried a proof-of-concept fix by saving a reference to the original SPI_tuptable and this seems to fix the crash and behave correctly:

--- a/topology/postgis_topology.c
+++ b/topology/postgis_topology.c
@@ -1975,6 +1975,7 @@ cb_updateTopoGeomFaceSplit ( const LWT_BE_TOPOLOGY* topo,
   StringInfoData sqldata;
   StringInfo sql = &sqldata;
   int i, ntopogeoms;
+  SPITupleTable *tupTable;
   const char *proj = "r.element_id, r.topogeo_id, r.layer_id, r.element_type";
 
   POSTGIS_DEBUGF(1, "cb_updateTopoGeomFaceSplit signalled "
@@ -2013,10 +2014,11 @@ cb_updateTopoGeomFaceSplit ( const LWT_BE_TOPOLOGY* topo,
   }
 
   ntopogeoms = SPI_processed;
+  tupTable = SPI_tuptable;
   for ( i=0; i<ntopogeoms; ++i )
   {
-    HeapTuple row = SPI_tuptable->vals[i];
-    TupleDesc tdesc = SPI_tuptable->tupdesc;
+    HeapTuple row = tupTable->vals[i];
+    TupleDesc tdesc = tupTable->tupdesc;
     int negate;
     int element_id;
     int topogeo_id;

[strk]

Thanks for the report ! Same applies to edge split. I'm working on automated tests.

[strk]

(In [14522]) Fix crash splitting faces used by multiple TopoGeometry objects

Thanks rulus for spotting and analyzing the bug. See #3407

[strk]

(In [14523]) Fix crash splitting faces used by multiple TopoGeometry objects

Thanks rulus for spotting and analyzing the bug. See #3407

[strk]

r14522 in trunk (2.3.0dev) and r14523 in 2.2 branch (2.2.1) fix the crash on face splitting. Edge splitting trunk is still todo.

[strk]

(In [14524]) Fix crash on splitting edge defining multiple TopoGeometries

Closes #3407

[strk]

(In [14525]) Fix crash on splitting edge defining multiple TopoGeometries

See #3407

[strk]

With r14524 (2.2.1) and r14525 (2.3.0) we also have the edge splitting crash fixed. Thanks again and happy holidays !