Bug#756983: Error in `osm2pgsql': free(): invalid pointer: 0x00007f4613a62678 ***

[Robert Edmonds]

Sebastiaan Couwenberg wrote:
> On 08/04/2014 05:59 PM, Robert Edmonds wrote:
> > I'd be happy to take a look and try to debug the issue to see if it's
> > caused by the protobuf-c changes.
> 
> Thanks, that's much appreciated.
> 
> > However, I'm entirely unfamiliar with osm2pgsql.  Is there any chance
> > you could walk me through setting up a test case, or provide a sample
> > file and command-line arguments that exhibit the problem?
> 
> The basic setup is described in the Operation section of the README file
> (/usr/share/doc/osm2pgsql/README.gz).
> 
> For my own test case I used a wheezy based database server:
> 
>  createdb osm
>  createlang plpgsql osm
>  psql osm < /usr/share/postgresql/9.1/contrib/postgis-1.5/postgis.sql
>  psql osm <
> /usr/share/postgresql/9.1/contrib/postgis-1.5/spatial_ref_sys.sql
>  psql -d osm -c "ALTER TABLE geometry_columns OWNER TO bas"
>  psql -d osm -c "ALTER TABLE spatial_ref_sys OWNER TO bas"
>  psql osm < ~/git/pkg-grass/osm2pgsql/900913.sql
>  echo "CREATE EXTENSION hstore" | psql osm
> 
> Adjust the paths for PostgreSQL 9.3 on sid.
> 
> OSM PBF files can be downloaded from Geofabrik:
> 
> http://download.geofabrik.de/
> 
> I used an old one I had lying around.
> 
> osm2pgsql command line I used:
> 
> osm2pgsql -c -l -s -d <dbname> -U <dbuser> -W -H <dbhost> -v
> netherlands-140201.osm.pbf

Thanks, I was able to figure out how to get an osm2pgsql setup working
with the above instructions and get a reproducible test case.

It looks like we (protobuf-c) were not explicitly zero'ing out memory in
certain cases, namely, when unpacking a 0-length byte string field in a
protobuf message.  (In the .osm.pbf file that I tried, this case
reliably appeared in the first element of the StringTable field in
PrimitiveBlock messages.)  This was causing a stray pointer to
occasionally be passed to free() when parse-pbf.c's processOsmData()
called primitive_block__free_unpacked().  Most of the time the affected
ProtobufCBinaryData object happens to be allocated with zero'd memory by
chance, which is why this bug wasn't caught sooner.

Here is the patch to protobuf-c.  I'll be releasing a new upstream
version of protobuf-c with this fix soon.

diff --git a/protobuf-c/protobuf-c.c b/protobuf-c/protobuf-c.c
index c7fb21d..b66ed93 100644
--- a/protobuf-c/protobuf-c.c
+++ b/protobuf-c/protobuf-c.c
@@ -2304,6 +2304,8 @@ parse_required_member(ScannedMember *scanned_member,
 			if (bd->data == NULL)
 				return FALSE;
 			memcpy(bd->data, data + pref_len, len - pref_len);
+		} else {
+			bd->data = NULL;
 		}
 		bd->len = len - pref_len;
 		return TRUE;

-- 
Robert Edmonds
edmonds at debian.org