Geometry output functions crash server with invalid WKT (windows 64)

[Mike Taves]

This is an oddity that can be demonstrated two ways on my work PC:

Read valid WKT followed by invalid WKT:

postgis=# SELECT ST_AsText('POINT(3 4)');
 st_astext
------------
 POINT(3 4)
(1 row)


postgis=# SELECT ST_AsText('POINT(3 4 hi)');
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.

Or read invalid WKT twice:

postgis=# SELECT ST_AsText('POINT Z(3 4 hi)');
ERROR:  parse error - invalid geometry
HINT:  "POINT Z(3 4 hi" <-- parse error at position 14 within geometry
CONTEXT:  SQL function "st_astext" statement 1
postgis=# SELECT ST_AsText('POINT Z(3 4 hi)');
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.

Note that only the second example shows a helpful error message.

POSTGIS="2.0.1 r9979" GEOS="3.3.5-CAPI-1.7.5" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.1, released 2012/05/15" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER

[Mike Taves]

Actually, this is a problem with ST_AsText, since the parse is robust:

postgis=# SELECT 'POINT(3 4 hi)'::geometry;
ERROR:  parse error - invalid geometry
LINE 1: SELECT 'POINT(3 4 hi)'::geometry;
               ^
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry
postgis=# SELECT 'POINT(3 4 hi)'::geometry;
ERROR:  parse error - invalid geometry
LINE 1: SELECT 'POINT(3 4 hi)'::geometry;
               ^
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry
postgis=# SELECT 'POINT(3 4 hi)'::geometry;
ERROR:  parse error - invalid geometry
LINE 1: SELECT 'POINT(3 4 hi)'::geometry;
               ^
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry

repeated, without crash.

[Mike Taves]

More testing ...

There is no crash if explicitly casted:

SELECT ST_AsText('POINT(3 4 hi)'::geometry);
SELECT ST_AsText('POINT(3 4 hi)'::geometry);

but this does crash on the second statement:

SELECT ST_AsText('POINT(3 4 hi)'::text);
SELECT ST_AsText('POINT(3 4 hi)'::text);

Going through the other output functions with the template:

SELECT <ST_function>('POINT(3 4 hi)'::text);
SELECT <ST_function>('POINT(3 4 hi)'::text);

Here are the results:

• ST_AsBinary - good
• ST_AsEWKB - good
• ST_AsEWKT - crash
• ST_AsGeoJSON - crash
• ST_AsGML - crash
• ST_AsHEXEWKB - good
• ST_AsKML - crash
• ST_AsSVG - crash
• ST_AsX3D - good
• ST_GeoHash - good
• ST_AsText - crash
• ST_AsLatLonText - good

[robe]

Mike, Which OS are you running on. I'm not seeing any crashing under my windows 64-bit 9.2 r my 32-bit 9.1. I tried 2.1.0SVN, and 2.0.1 on 32-bit.

With query:

SELECT ST_AsText('POINT(3 4 hi)');

It just gives this notice

ERROR:  parse error - invalid geometry
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry
CONTEXT:  SQL function "st_astext" statement 1

SELECT ST_AsText('POINT Z(3 4 hi)');

-- gives notice

ERROR:  parse error - invalid geometry
HINT:  "POINT Z(3 4 hi" <-- parse error at position 14 within geometry
CONTEXT:  SQL function "st_astext" statement 

Tested on these

PostgreSQL 9.2.2, compiled by Visual C++ build 1600, 64-bit POSTGIS="2.1.0SVN r11008" GEOS="3.4.0dev-CAPI-1.8.0 r0" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.1, released 2012/05/15" LIBXML="2.7.8" LIBJSON="UNKNOWN" (core procs from "2.1.0SVN r11004" need upgrade) RASTER (raster procs from "2.1.0SVN r11004" need upgrade)

PostgreSQL 9.1.4, compiled by Visual C++ build 1500, 32-bit POSTGIS="2.0.1 r9979" GEOS="3.3.5-CAPI-1.7.5" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.1, released 2012/05/15" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER

[robe]

could be related to this ticket which went away when user upgraded. #2180

[robe]

I just upgraded my 64-bit to 9.1.7

PostgreSQL 9.1.7, compiled by Visual C++ build 1500, 64-bit POSTGIS="2.0.1 r9979" GEOS="3.3.5-CAPI-1.7.5" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.1, released 2012/05/15" LIBXML="2.7.8" LIBJSON="UNKNOWN" (core procs from "2.0.0 r9605" need upgrade) RASTER (raster procs from "2.0.0 r9605" need upgrade)

And:

SELECT ST_AsText('POINT(3 4 hi)');

ERROR:  parse error - invalid geometry
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry
CONTEXT:  SQL function "st_astext" statement 1

Behaves as expected.

Unfortuantely I never downloaded 9.1.4 binaries, but I suspect 9.1.4 might ahve been a bad build since #2180 was running 9.1.4 and I'm not seeing the issue on 9.1.3 or 9.1.7.

Mike -- can you confirm what OS and build of PostgreSQL you are running. If you are running 9.1.4 I'd like to close this out as an upstream (probably already fixed) issue.

[Mike Taves]

I have two versions of PostgreSQL and PostGIS installed on my work PC on Windows 7 x64. Both were installed by Application Stack Builder at different times.

Interestingly they have different results, in summary:

• PostgreSQL 9.1.3 / PostGIS 2.0.0 r9605 - works
• PostgreSQL 9.2.2 / PostGIS 2.0.1 r9979 - crashes (used for this bug reporting)

Here are more details from both.

postgis=# SELECT version();
                           version
-------------------------------------------------------------
 PostgreSQL 9.1.3, compiled by Visual C++ build 1500, 64-bit
(1 row)


postgis=# SELECT postgis_full_version();
                                                                         postgis_full_version

-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
 POSTGIS="2.0.0 r9605" GEOS="3.3.3-CAPI-1.7.4" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.0, released 2011/12/29" LIBXML="2.7.8" LIBJSON="UNKNOWN" TOPOLOGY RASTER
(1 row)


postgis=# SELECT ST_AsText('POINT(3 4 hi)'::text);
ERROR:  parse error - invalid geometry
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry
CONTEXT:  SQL function "st_astext" statement 1
postgis=# SELECT ST_AsText('POINT(3 4 hi)'::text);
ERROR:  parse error - invalid geometry
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry
CONTEXT:  SQL function "st_astext" statement 1

no crash, works as expected

postgis=# SELECT version();
                           version
-------------------------------------------------------------
 PostgreSQL 9.2.2, compiled by Visual C++ build 1600, 64-bit
(1 row)


postgis=# SELECT postgis_full_version();
NOTICE:  Function postgis_topology_scripts_installed() not found. Is topology support enabled and topology.sql installed?
                                                                     postgis_full_version
--------------------------------------------------------------------------------------------------------------------------------------------------------------
 POSTGIS="2.0.1 r9979" GEOS="3.3.5-CAPI-1.7.5" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.1, released 2012/05/15" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER
(1 row)


postgis=# SELECT ST_AsText('POINT(3 4 hi)'::text);
ERROR:  parse error - invalid geometry
HINT:  "POINT(3 4 hi" <-- parse error at position 12 within geometry
CONTEXT:  SQL function "st_astext" statement 1
postgis=# SELECT ST_AsText('POINT(3 4 hi)'::text);
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.

[robe]

ah interesting. I just redownloaded the executable I packaged for 2.0.1 9.2.2 after on my windows 7 after 3 calls it does crash the server. So at least I can replaicte it. I'm going to see if it happens with 2.0.2.

I'm running 2.1.0SVN on my windows 2003/2008 64-bit in production because I needed the new raster features and speed improvements for geography and those seem to work fine. Could be my packaging of that install too I guess.

[robe]

Hmm does it on my 2.0.2 build too :(.

[robe]

hmm boy this is complicated. I just tried the 2.1.0SVN build on my windows 7 box 9.2.2 64-bit and it crashes too.

However the same build on my windows 2003 64-bit 9.2.2 works just dandy. ON another windows 2008 64-bit I have running 9.2.2 2.0.1 it exhibits the same crashing.

[robe]

I'll recompile against 9.2.2 to see if that makes a difference I might have been compiling again 9.2.1 or something. Anyrate have the same issue on another Windows 2008 64-bit server I just tried running 2.1.0 SVN. so at least seems like its probably an issue on windows 2008 and windows 7 but not on windows 2003 64-bit for some reason.

[robe]

I'm beginning to think this is a bug in PostgreSQL rather than PostGIS. What is really really odd why I think that is that the ST_AsText function called with unknown is nothing but this:

CREATE OR REPLACE FUNCTION st_astext(text)
  RETURNS text AS
' SELECT ST_AsText($1::geometry);  '
  LANGUAGE sql IMMUTABLE STRICT
  COST 100;

And yet if you call:

SELECT ST_AsText('POINT(3 4 hi)'::geometry);

IF doesn't crash. How bizarre is that? It's almost like the IMMUTABLE STRICT is having a weird side effect.

pramsey -- I thought we had an issue like this where wrapping logic in a fucntion would craash but calling the same logic explicitly wouldn't, but can't recall what that ticket was. Does that sound familiar?

[robe]

more sleuthing yuck. I think it is the same issue as #1999 which I thought we had fixed, but I guess wishful thinking

[robe]

Yap immuatable is playing a curious role in this bug. I created a version of the ST_AsText without IMMUTABLE:

CREATE OR REPLACE FUNCTION st_astextNotImmut(text)
  RETURNS text AS
' SELECT ST_AsText($1::geometry);  '
  LANGUAGE sql STRICT
  COST 100;

and it does not crash. I can call this as many times to my hearts content.

SELECT ST_AsTextNotImmut('POINT Z(3 4 hi)');

and no crashing. So I guess as a temporary fix you could remove the IMMUTABLE by redefining as this.

CREATE OR REPLACE FUNCTION ST_AsText(text)
  RETURNS text AS
' SELECT ST_AsText($1::geometry);  '
  LANGUAGE sql STRICT
  COST 100;

until we figure out what exactly immutable is doing to cause the crash.

[darkblueb]

I conferred briefly with Andres on #postgresql 13may13. Andres suggested that the bug be filed on PostgreSQL, even better with a stack trace.

[robe]

Thanks darkblueb. Unfortunately its hard to give a stack trace for this one because it only happens on windows 2008/windows 7-64bit for a particular version of PostgreSQL (as I recall like 9.2.something) or was it 9.1.something under EDB run (not my mingw64 where I normally do stack traces) which gives me pretty useless information. It doesn't happen for windows 2003 64-bit for example. Anyway I'm also checking to see if its an issue on 9.3beta1.

[robe]

The good news in the 9.3beta1 windows 7 64-bit loaded with PostGIS 2.1.0beta2 doesn't have this issue. Yeh.

[robe]

Bad news its still an issue in 9.2.4 EDB windows 7 running 2.0.3. So sems isolated to something about 9.2. I also tried on my older 9.2.2 and had same issue.

SELECT ST_AsText('POINT(3 4 hi)'); -- crash

SELECT postgis_full_version() || ' ' || version();

-- POSTGIS="2.0.3 r11132" GEOS="3.4.0dev-CAPI-1.8.0 r0" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.10.0, released 2013/04/24" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER PostgreSQL 9.2.2, compiled by Visual C++ build 1600, 64-bit

and also
POSTGIS="2.0.3 r11132" GEOS="3.3.8-CAPI-1.7.8" PROJ="Rel. 4.8.0, 6 March 2012" GDAL="GDAL 1.9.2, released 2012/10/08" LIBXML="2.7.8" LIBJSON="UNKNOWN" RASTER PostgreSQL 9.2.4, compiled by Visual C++ build 1600, 64-bit

I'll see if I can come up with a function that doesn't involve postgis to crash this thing to make it easier for upstream to debug and to rule out postgis as the culprit.

[robe]

For reference posted the issue upstream:

​http://www.postgresql.org/message-id/E1Uc6tz-0006eK-WE@wrigleys.postgresql.org

bug 8156

[robe]

see #2332 for a similar possibly related case

[Bborie Park]

I can't recreate this using 2.0 branch on Windows 7 64-bit with EDB 9.2.2.

[Bborie Park]

Nor with EDB 9.2.4.

[robe]

Really. I was able to on my windows 2008 x64bit running PostgreSQL 9.2.4 EDB 64-bit with latests branch (I need to retry latest branch on my windows 7). You are testing with 64-bit PostgreSQL EDB right? Definitely doesn't happen on 32-bit PostgreSQL install even 32-bit running on windows 7/windows 2008 64-bit. I'll give it another try on windows 7. Also doesn't seem to trigger on windows 2003 x64bit 9.2.4

[Bborie Park]

Correct. I'm using EDB 9.2.4 x86-64 that I downloaded today.

[Mike Taves]

I just did a fresh install of postgresql-9.2.4-1-windows-x64.exe on a Win7 Enterprise-x64 computer, and I recreated the exact same crash initially reported.

Would a locale make any difference? Both my system and database locale always some sort of Commonwealth locale, like 'English_New Zealand.1252', but never 'English_United States.1252' or C (sans-locale).

... I uninstalled/reinstalled and initialised with a C local, and I get the same crash.

[Bborie Park]

I finally got the crash to happen. Just calling the following twice won't do it. Using the SQL statement twice in the same session does cause a crash..

[robe]

I think this is a similar issue as I fixed in #2332. The fundamental problem is with the geometry(TEXT) (which has underlying c function: parse_WKT_lwgeom ) ​http://postgis.net/docs/doxygen/2.1/d9/d6e/lwgeom__inout_8c_a1b3eab5b08e6e0ccec4b8a2889e6bb8c.html#a1b3eab5b08e6e0ccec4b8a2889e6bb8c which does a similar call to yet another PG function.

Repeating all that code is too much and why it even bothers when it does nothing but pass along the result is beyond me. I'm sure there's a whole bunch of leaky memory in here corrupting shared memory which 9.2 EDB 64-bit is especially sensitive too.

[robe]

I was wondering why if in fact geometry(TEXT) is the culprit ,why it doesn't crash like the others. The reason is simple since the cost of it is set to 1 (because its a C func and we set no costs) it never gets cached. The SQL functions on the other hand since we weren't setting costs were set to 100 presumably a threshold that 9.2edb x64 thought worthy of caching.

I committed a less than honorable patch at r11476 for trunk (just because the costs are set too high anyway and it happens to as a side effect sugar coat this issue) and technicallly no one should be using these functions since they are there just to prevent text reps from falling into the geography trap.

I'm going to experiment and see if I set the costs high enough under ming64 (which doesn't crash), if I can trigger the crash. Would be interesting to see if increasing the cost would trigger the issue on other systems.

[robe]

well it was crashing still sometimes on my 9.2. I ended up just removing the immutable entirely at r11477 . these functions are deprecated and should be removed in the next major release we have (possibly sooner).

[Bborie Park]

So, the problem boils down to a buffer overflow. I've attached the outputs of WinDbg.

[Bborie Park]

Stack trace from WinDbg

[robe]

dustymugs -- thanks. FWIW I've isolated the problem areas down to:

​http://postgis.net/docs/doxygen/2.1/d9/d6e/lwgeom__inout_8c_source.html#l00066 (line 66 of lwgeom_inout)

SELECT ST_AsText('');  -- will trigger that part

and I can prevent it from crashing if I replace:

   //ereport(ERROR,(errmsg("parse error - invalid geometry")));
   lwerror("parse error2 - invalid geometry");
   PG_RETURN_NULL();

It also triggers at: ​http://postgis.net/docs/doxygen/2.1/d9/d6e/lwgeom__inout_8c_source.html#l00108

 if ( lwgeom_parse_wkt(&lwg_parser_result, str, LW_PARSER_CHECK_ALL) == LW_FAILURE )
    {
         PG_PARSER_ERROR(lwg_parser_result);
      }

For the inputs that mwtoews mentioned. That I haven't bothered yet trying to fiddle with.

Other observation -- if I set the functions to STABLE (instead of IMMUTABLE) that also seems to be as good as VOLATILE. So maybe its because the STABLE memory event though its polluted may not matter since its thrown away anyway.

[robe]

Actually I think the trick is the PG_RETURN_NULL. I can prevent the leak by changing that line to a

 if ( lwgeom_parse_wkt(&lwg_parser_result, str, LW_PARSER_CHECK_ALL) == LW_FAILURE )
    {
         PG_PARSER_ERROR(lwg_parser_result);
         PG_RETURN_NULL();
      }

Though I had assumed the PG_PARSER_ERROR would exit the function so surprised that makes a difference. You see an issue with just returning PG_RETURN_NULL() in these cases?

[Bborie Park]

PG_PARSER_ERROR does exit the function so PG_RETURN_NULL() would be adequate. BUT, you'd lose the following...

HINT:  "POINT Z(3 4 hi" <-- parse error at position 14 within geometry

So, we'll need to debug pg_parser_errhint() found in libpgcommon/lwgeom_pg.c.

[robe]

Went ahead and committed to 2.0 branch at r11481 . I can't trigger these in my 2.0 anymore and it fixed two of the failing regress scripts (now like 5 more to go I think).

I'm going to revert my change in 2.1 and replace with this one if debbie seemscontent with the changes.

I don't seem to loose the hint -- here is what I get now on my 9.2 edb x64 PostGIS 2.0.4 SVN

SELECT ST_AsText('POINT Z(3 4 i)');

ERROR:  parse error - invalid geometry
HINT:  "POINT Z(3 4 i)" <-- parse error at position 14 within geometry
CONTEXT:  SQL function "st_astext" statement 1

and I can now run as many times as I want without it crashing the service :)

[robe]

Okay finally I think I can close this one out i tried on a couple of crasher functions posed. last fix at r11482 . So should now be fixed for both 2.0 and 2.1 branches.

I neglected to mention that I don't think this ever happened in windows 2003 9.2 edb 64-bit (or at least as I recall I couldn't trigger it) so I suspect it may have been also memory specific that on windows 7 and windows 2008 because of the costing metrics it was more likely to fall into shared memory where the infection happens.

mwtoews, If you can retest out (winnie is done building both branches I think), that would be swell.

Now off to kill more windows 64-bit crashers :)

[Bborie Park]

So strange. ereport(ERROR, ...) is not supposed to return to the calling function so PG_RETURN_NULL() shouldn't be needed. But adding PG_RETURN_NULL() prevents the issue. Strange...

​http://doxygen.postgresql.org/elog_8h_source.html#l00084

[robe]

Might be a similar issue on #2340 that one since its a void function doesn't make sense to put in a PG_RETURN anything.

[Mike Taves]

I'm not sure how to check-up on the status of Winnie's builds, but ​http://winnie.postgis.net/download/windows/pg92/buildbot/postgis-pg92-binaries-2.0.4SVNw64.zip is incomplete, as it is missing the libraries. lib is empty only for postgis-pg92-binaries-2.0.4SVNw64.

[pramsey]

If I may stand in for strk here: this seems like a very subtle problem with a very subtle fix, and likely to be accidentally reverted at some point in the future. Therefor: is there a test in place for this, that crashes without this fix but doesn't with it?

[robe]

Not that I can exercise without windows 64-bit testing under EDB. Without this regress ticket fails (but only under EDBB). I can maybe put a more explicit description in those sections like "strk PLEASE DON'T remove this no matter how much you hate windows users" to grab his attention.

As dustymugs noted though in theory this patch should not be needed so seems like something else

[robe]

mwtoews,

Rebuilding now. The two jobs might have crossed each other. Try in another hour or so. When I tested the winnie package I was only testing the 2.1 one.

[robe]

speaking of which -- what does this little snippet mean:

​http://doxygen.postgresql.org/elog_8h_source.html#l00043 (note that EDB Windows 32 doesn't have these kind of problems just the 64-bit)

43 /* Save ERROR value in PGERROR so it can be restored when Win32 includes
44  * modify it. We have to use a constant rather than ERROR because macros
45  * are expanded only when referenced outside macros.
46  */
47 #ifdef WIN32
48 #define PGERROR 20
49 #endif
50 #define FATAL 21 /* fatal error - abort process */
51 #define PANIC 22 /* take down the other backends with me */
52 

[robe]

mwtoews,

oops sorry about that. I had the 9.2x64 2.0 package process building against a geos version I deleted so failed on build. Should be fixed now.

[Mike Taves]

Testing with r11481 does not crash the server, and the expected error message is shown as many times as needed.

[Bborie Park]

As pramsey indicates, this is a subtle bug. The interaction between PG_PARSER_ERROR() and PG_RETURN_NULL() somehow works around the issue. What's super strange is that if you were to try...

PG_PARSER_ERROR(lwg_parser_result);
elog(NOTICE, "I'm a little teapot");
PG_RETURN_NULL();

the NOTICE will not fire.

I can't seem to find a way to run a debugger with EDB and PostGIS which is quite... annoying... more experimenting to do... maybe substitute the actual code for the macros.

[robe]

Might be easier to debug the ereport one: Note: I put PG_RETURN_NULL(); in two spots in r11481.

The first spot was to prevent ST_AsText() from crashing and that one doesn't even go thru the parser -- just uses ereport. A lot of the crashes I've been seeing so far usually occur when an ereport(ERROR) is done and as you said in theory no return should need to be made.

[Bborie Park]

robe2,

Can you back out your changes for this ticket and see if everything behaves correctly with gcc 4.8?

[robe]

Yap works fine with 4.8 testing against EDB 9.3 beta 1 x64-bit on my Windows 7 64-bit if I back out my changes. Though have to leave these in since I'll still be using 4.5.4 for 9.2/9.1 (9.0 PostgreSQL as noted here ​http://www.postgresql.org/message-id/14242.1365200084@sss.pgh.pa.us ) I don't even think supports compiling with gcc 4.8.0 then again I'm not sure any windows users use 64-bit 9.0 and if they do if 9.0 even has the issue seems to be more of an issue with VS 2010 than VS 2008 I think perhaps because VS 2008 is closer in family to 4.5.4. That might also be why for 9.1 I've only seen the issue under windows 2008/windows 7.0 64-bit (not my windows 2003 64-bit) since the newer windows OS are more likely to have external dependencies built with newer VS 2010 that PostgreSQL relies on (even when running a 9.1 built with VS 2008).