Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIGSEGV with native C extension when IP queried is null/empty string #96

Closed
proton-ab opened this issue Dec 14, 2019 · 17 comments
Closed

Comments

@proton-ab
Copy link

As simple as that.

[14-Dec-2019 09:27:16] WARNING: [pool www] child 2662 exited on signal 11 (SIGSEGV) after 133.373968 seconds from start

Example code:

$res = $this->reader->country($ip)->country->isoCode;
@oschwald
Copy link
Member

I get InvalidArgumentException: The value "" is not a valid IP address. Could you provide the output of php -i?

@proton-ab
Copy link
Author

proton-ab commented Dec 14, 2019

You are right, I can not reproduce it with simple code. Try this instead:

#!/usr/bin/env php
<?php

define('BASE_ROOT', __DIR__);
require_once BASE_ROOT . '/vendor/autoload.php'; // set up autoloading

use Exception;
use GeoIp2\Database\Reader;

class GeoIP
{
  private $reader;

  public function __construct()
  {
    $this->reader = new Reader('GeoLite2-Country.mmdb');
  }

  public function __destruct()
  {
    $this->reader->close();
  }

  public function getCc($ip)
  {
    try {
      $res = $this->reader->country($ip)->country->isoCode;
    } catch (Exception $e) {
      $res = null;
    }

    return $res ? $res : '-';
  }
}

$geoIp = new GeoIP();

var_dump($geoIp->getCc(''));

I get this when running under GDB. This does not happen if extension is not installed.

Starting program: /usr/bin/php debug.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
string(1) "AU"
string(1) "-"
string(1) "-"
string(1) "-"
string(1) "-"

Program received signal SIGSEGV, Segmentation fault.
__GI___libc_free (mem=0xc0000f18fffffffd) at malloc.c:3093

@proton-ab
Copy link
Author

proton-ab commented Dec 14, 2019

Please find attached output of php -i from Vagrant instance where I've tested the script above to fail.

phpinfo.vagrant.txt

In case it's of importance, we're using older version of jemalloc than available in Buster repos. (Debian refers to it as libjemalloc1 as opposed to libjemalloc2)

root@buster:/code# dpkg -l | grep malloc
ii  libjemalloc1                          3.6.0-9.1                     amd64        general-purpose scalable concurrent malloc(3) implementation
ii  libjemalloc1-dbg                      3.6.0-9.1                     amd64        debug symbols for jemalloc
echo
echo Installing libjemalloc1
echo 'deb-src http://ftp.nl.debian.org/debian/ stretch main contrib non-free' > /etc/apt/sources.list.d/jemalloc.list
apt-get update
cd /opt
apt-get -y install build-essential fakeroot devscripts lintian
apt-get -y source libjemalloc1
apt-get -y build-dep libjemalloc1
cd jemalloc-3.6.0
debuild -us -uc -i -I
debi
apt-get -y remove libjemalloc-dev

@oschwald
Copy link
Member

Thanks. I am still unable to reproduce it with the provided script and the Ubuntu PHP 7.3 package (7.3.11-0ubuntu0.19.10.1), but I will do some more testing. I see that you are on 1.51. Did this happen with older versions of the extension?

Could you also get a backtrace from gdb?

@proton-ab
Copy link
Author

No, this did not happen on 1.4.1 with libmaxminddb 1.3.2. Of note is also the fact that upgrading to 1.5.1 from 1.5.0 breaks compatibility with extension below 1.5.0 due to usage of getWithPrefixLen.

Note that SIGSEGV happens only when close() is called on reader instance, so in case of my example, on __destruct()

Here's backtrace you asked for.

(gdb) r
Starting program: /usr/bin/php debug.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
string(1) "-"

Program received signal SIGSEGV, Segmentation fault.
__GI___libc_free (mem=0xc571c861e168ada9) at malloc.c:3093
3093	malloc.c: No such file or directory.
(gdb) bt
#0  __GI___libc_free (mem=0xc571c861e168ada9) at malloc.c:3093
#1  0x00007ffff18fd15e in free_mmdb_struct (mmdb=0x7ffff5116840) at maxminddb.c:1848
#2  0x00007ffff18fe7d5 in MMDB_close (mmdb=<optimized out>) at maxminddb.c:1838
#3  0x00007ffff2853cf4 in zim_MaxMind_Db_Reader_close (execute_data=<optimized out>, return_value=<optimized out>) at /opt/MaxMind-DB-Reader-php/ext/maxminddb.c:397
#4  0x00005555558813fe in execute_ex ()
#5  0x00005555557edd4e in zend_call_function ()
#6  0x000055555582be3d in zend_objects_destroy_object ()
#7  0x0000555555830e29 in zend_objects_store_del ()
#8  0x000055555580c2d0 in zend_hash_reverse_apply ()
#9  0x00005555557ec975 in ?? ()
#10 0x00005555557fb8e5 in zend_call_destructors ()
#11 0x000055555579b0fd in php_request_shutdown ()
#12 0x00005555558849fa in ?? ()
#13 0x0000555555661aaf in ?? ()
#14 0x00007ffff745209b in __libc_start_main (main=0x555555661620, argc=2, argv=0x7fffffffe688, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe678) at ../csu/libc-start.c:308
#15 0x0000555555661baa in _start ()

@proton-ab
Copy link
Author

proton-ab commented Dec 14, 2019

Here's another backtrace for mutation of script:

#0  __GI___libc_free (mem=0xc571c861e168ada9) at malloc.c:3093
#1  0x00007ffff18fd15e in free_mmdb_struct (mmdb=0x7ffff5114840) at maxminddb.c:1848
#2  0x00007ffff18fe7d5 in MMDB_close (mmdb=<optimized out>) at maxminddb.c:1838
#3  0x00007ffff2853cf4 in zim_MaxMind_Db_Reader_close (execute_data=<optimized out>, return_value=<optimized out>) at /opt/MaxMind-DB-Reader-php/ext/maxminddb.c:397
#4  0x00005555558813fe in execute_ex ()
#5  0x00005555557edd4e in zend_call_function ()
#6  0x000055555582be3d in zend_objects_destroy_object ()
#7  0x0000555555830e29 in zend_objects_store_del ()
#8  0x0000555555871995 in ?? ()
#9  0x000055555587ad25 in execute_ex ()
#10 0x0000555555883057 in zend_execute ()
#11 0x00005555557fbc93 in zend_execute_scripts ()
#12 0x000055555579c458 in php_execute_script ()
#13 0x00005555558855be in ?? ()
#14 0x0000555555661aaf in ?? ()
#15 0x00007ffff745209b in __libc_start_main (main=0x555555661620, argc=2, argv=0x7fffffffe688, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe678) at ../csu/libc-start.c:308
#16 0x0000555555661baa in _start ()
var_dump($geoIp->getCc(''));
unset($geoIp); <-- SIGSEGV here
var_dump('Yay!');

@oschwald
Copy link
Member

Even with the unset, it seems to work for me:

string(1) "-"
string(4) "Yay!"

Based on the trace, I'd guess the issue is libmaxminddb rather than the extension, but the problem isn't clear to me after looking at the relevant code.

@proton-ab
Copy link
Author

proton-ab commented Dec 14, 2019

Yes the problem seems to lie with libmaxminddb, specifically here: https://github.com/maxmind/libmaxminddb/blob/master/src/maxminddb.c#L1848. You own mmdb->filename so it's unlikely PHP would have anything to do with it. Are you able to perform test on Debian Buster with and without libjemalloc1 installed as per instructions above?

@oschwald
Copy link
Member

I haven't tried setting up a Buster environment yet. Looking at the libmaxminddb code, it seems correct. The only thing that I can think of is that somehow MMDB_close is being called on an uninitialized mmdb. To that end, could you try this change?

@proton-ab
Copy link
Author

proton-ab commented Dec 14, 2019

Sadly this does not solve the issue.

I've tested this without libjemalloc1 on Buster (by removing it and recompiling lib and extension) and it seems to happen anyway, so most likely not a culprit.

@oschwald
Copy link
Member

That's too bad. I am trying to build a Buster environment that matches yours. How did you install libmaxminddb? Buster seems to only have 1.3.2.

@proton-ab
Copy link
Author

Directly from source as per instructions

wget https://github.com/maxmind/libmaxminddb/releases/download/1.4.2/libmaxminddb-1.4.2.tar.gz
tar -xvzf libmaxminddb-1.4.2.tar.gz
cd libmaxminddb-1.4.2
./configure
make
make install

@oschwald
Copy link
Member

oschwald commented Dec 14, 2019

Unfortunately, I am still unable to reproduce it:

# php debug.php 
PHP Warning:  The use statement with non-compound name 'Exception' has no effect in /root/segfault/debug.php on line 7
string(1) "-"
string(4) "Yay!"

Using the debian:buster-backports Docker image with php7.3 at 7.3.11-1~deb10u1 with the GeoLite-Country.mmdb built on 2019-12-10 15:06:53 UTC.

php -i output.

@proton-ab
Copy link
Author

proton-ab commented Dec 14, 2019

Hi again,

Sorry for late response, this took way too long to debug than I expected. So, the issue looks like memory corruption but it manifests only when sufficient stuff is happening in PHP, for example due to PSR-0.

For example, please try using this composer.json to install required packages:

{
    "name": "proton-ab/maxmind",
    "require": {
        "php": "~7.3",
        "slim/slim": "^3.12",
        "paragonie/csp-builder": "^2.4",
        "paragonie/anti-csrf": "^2.2",
        "spomky-labs/otphp": "^10.0",
        "endroid/qr-code": "^3.7.3",
        "robmorgan/phinx": "^0.9.2",
        "slim/twig-view": "^2.5",
        "slim/flash": "^0.4.0",
        "dflydev/fig-cookies": "^2.0",
        "geoip2/geoip2": "~2.10.0"
    }
}

For me it ends up with this:

(gdb) r
Starting program: /usr/bin/php debug.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
string(1) "-"
free(): invalid pointer

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7451535 in __GI_abort () at abort.c:79
#2  0x00007ffff74a8508 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7ffff75b328d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3  0x00007ffff74aec1a in malloc_printerr (str=str@entry=0x7ffff75b143b "free(): invalid pointer") at malloc.c:5341
#4  0x00007ffff74b042c in _int_free (av=<optimized out>, p=<optimized out>, have_lock=<optimized out>) at malloc.c:4165
#5  0x00007ffff1b3215e in free_mmdb_struct (mmdb=0x7ffff105cdc0) at maxminddb.c:1848
#6  0x00007ffff1b337d5 in MMDB_close (mmdb=<optimized out>) at maxminddb.c:1838
#7  0x00007ffff27a9cf4 in zim_MaxMind_Db_Reader_close (execute_data=<optimized out>, return_value=<optimized out>) at /opt/MaxMind-DB-Reader-php-1.5.1/ext/maxminddb.c:397
#8  0x00005555558813fe in execute_ex ()
#9  0x00005555557edd4e in zend_call_function ()
#10 0x000055555582be3d in zend_objects_destroy_object ()
#11 0x0000555555830e29 in zend_objects_store_del ()
#12 0x000055555580c2d0 in zend_hash_reverse_apply ()
#13 0x00005555557ec975 in ?? ()
#14 0x00005555557fb8e5 in zend_call_destructors ()
#15 0x000055555579b0fd in php_request_shutdown ()
#16 0x00005555558849fa in ?? ()
#17 0x0000555555661aaf in ?? ()
#18 0x00007ffff745309b in __libc_start_main (main=0x555555661620, argc=2, argv=0x7fffffffe698, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe688) at ../csu/libc-start.c:308
#19 0x0000555555661baa in _start ()

Adding

"guzzlehttp/psr7": "^1.6"

at the end causes SIGSEGV.

Here's list of packages I install for debug environment:

apt-get -y install software-properties-common php7.3 php7.3-common php7.3-dev libcurl3-openssl-dev build-essential php7.3-cli libpq5 libodbc1 zip unzip curl wget php7.3-curl php7.3-mysqlnd php7.3-bcmath php7.3-gd php7.3-imagick php7.3-memcached php7.3-zip php7.3-mbstring

@oschwald
Copy link
Member

Thanks. I just got back to this and I am now able to reproduce the issue.

@oschwald
Copy link
Member

Would you be able to test #98 to check that it resolves the issue for you?

@proton-ab
Copy link
Author

This does indeed fix the issue. Thanks.

@horgh horgh closed this as completed in 5b373ae Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants