Segmentation fault on alpine

[Jáchym Toušek]

Thread 1 "php" received signal SIGSEGV, Segmentation fault.
0x00007fe2a24462df in __stack_chk_fail () from /lib/ld-musl-x86_64.so.1
(gdb) bt
#0  0x00007fe2a24462df in __stack_chk_fail () from /lib/ld-musl-x86_64.so.1
#1  0x00007fe29f6536ee in php_event_zval_to_fd (pfd=0x7fe2a1a16420) at /tmp/pear/temp/event/php7/src/util.c:65
#2  0x00007fe29f654882 in zim_Event___construct (execute_data=<optimized out>, return_value=<optimized out>) at /tmp/pear/temp/event/php7/classes/event.c:264
#3  0x00005597612c6bed in execute_ex ()
#4  0x00005597612c80ed in zend_execute ()
#5  0x0000559761242744 in zend_execute_scripts ()
#6  0x00005597611e4428 in php_execute_script ()
#7  0x00005597612ca175 in do_cli ()
#8  0x0000559760f916a7 in main ()

‌

[Ruslan Osmanov]

@Jáchym Toušek, could you add the steps to reproduce?

[Jáchym Toušek]

@Ruslan Osmanov Unfortunately not since it happens with proprietary code.

‌

[Jáchym Toušek]

I commented out some of my code and figured out that it happens when I execute a simple query against Postgres database using https://github.com/amphp/postgres. I can try to create a simple repository that might hopefully reproduce it but please make sure you really need it. It would take some time so I don’t wanna do it unnecessarily.

[Ruslan Osmanov]

Looks like the code tried to construct Event based on a closed PHP stream. Although, I admit that segmentation fault is a clear sign of internal error. But I’m not sure it is directly caused by pecl-event. The backtrace refers to the part that simply calls standard PHP streams API:

            } else if (php_stream_can_cast(stream, PHP_STREAM_AS_STDIO | PHP_STREAM_CAST_INTERNAL) == SUCCESS) {
                if (php_stream_cast(stream, PHP_STREAM_AS_STDIO,
                            (void *) &file_desc, 1) != SUCCESS || file_desc < 0) {
                    return -1;
                }

I could dig into this further, if I had more information. Without a minimal reproducible example, this ticket may hang here indefinitely, I’m afraid.

[Jáchym Toušek]

Alright, I’ll try to create a reproducer then. Thanks!

[Jáchym Toušek]

https://github.com/enumag/event-segfault

Steps to reproduce:

docker-compose up -d
docker-compose run api sh
php run.php

This should print `Segmentation fault (core dumped)`.

To get gdb backtrace:

docker-compose up -d
docker-compose run api sh
gdb php
run /usr/app/run.php
bt

‌

[Jáchym Toušek]

Any news on this?

[Ruslan Osmanov]

Any news on this?

Sorry, I’have no much free time lately. The reason of the segfault is that the user tries to add an Event which seems to be either freed, or constructed improperly. The following patch should prevent the segfault:

diff --git a/php7/classes/event.c b/php7/classes/event.c
index d0b01a1..868d3e3 100644
--- a/php7/classes/event.c
+++ b/php7/classes/event.c
@@ -451,6 +451,11 @@ PHP_METHOD(Event, add)

        e = Z_EVENT_EVENT_OBJ_P(zevent);

+       if (!e->event) {
+               php_error_docref(NULL, E_WARNING, "Failed adding event: Event object is either not initialized properly, or freed");
+               RETURN_FALSE;
+       }
+
        if (timeout == -1) {
                res = event_add(e->event, NULL);
        } else {

But from what I can see, the Event objects are not freed (Event::free()) before Event::add is called. Otherwise, I don’t see how the underlying event object (e->event) could be reset to zero.

Event objects can be constructed using one of the following methods:

• Event::__construct
• Event::timer
• Event::signal

Event::__construct raises a fatal error when it fails to create an event with event_new(). Event::timer and Event::signal do similar checks and return false in case of an error. So I can’t see how Event::add could be called before the Event object is constructed properly (race condition? )

Maybe you have ideas.

[Ruslan Osmanov]

Committed the fix to master. Could upload this to the PECL as well.

But I’m still curious how e->event could be reset to zero, if Event::free is not called before Event::add

[Jáchym Toušek]

Unfortunately I don’t know the internals of `\Amp\Loop\EventDriver` so I don’t know. I created an issue in amphp/postgres - kelunik or trowski might have some idea. https://github.com/amphp/postgres/issues/28

[Ruslan Osmanov]

Uploaded version 2.5.5 to PECL.

[Jáchym Toušek]

@Ruslan Osmanov If you can push the fix to pecl then I can try again to see if the newly added error pops up. If it does, it might give us some more ideas what’s wrong.

[Jáchym Toušek]

Ah, okay, lemme try.

[Jáchym Toušek]

@Ruslan Osmanov Even with ext-event v2.5.5 I’m still getting the same segfault and same gdb stacktrace.

https://github.com/enumag/event-segfault/commit/65dbc630074a122a7570e4e3b46adbb3f13174d3

[Ruslan Osmanov]

Oh, sorry. That’s yet another one. I’ve actually fixed another bug. Okay.

[Ruslan Osmanov]

Just for record. Tried to reproduce with a manual debug build, but the bug doesn’t appear this way:

sudo docker-compose down
sudo docker-compose build --no-cache --force-rm

docker-compose run api sh

apk add php7-dev php7-sockets
apk add libc-dev gcc git openssl openssl-dev libevent libevent-dev make autoconf pkgconfig


# Manually build pecl-event
git clone https://[e-mail]/osmanov/pecl-event.git 
cd pecl-event
phpize --clean
phpize

mkdir -p /usr/local/include/php/ext/sockets/
cp /usr/include/php7/ext/sockets/php_sockets.h /usr/local/include/php/ext/sockets/php_sockets.h

./configure --prefix=/usr --with-event-core --with-event-extra --with-event-openssl --enable-event-debug 

make clean
make -j3
make install

php ../run.php

ext-event version: 2.5.5

Warning: Event::add(): Failed adding event in /usr/app/vendor/amphp/amp/lib/Loop/EventDriver.php on line 348

Warning: Event::add(): Failed adding event in /usr/app/vendor/amphp/amp/lib/Loop/EventDriver.php on line 348

Fatal error: Uncaught Amp\TimeoutException: Operation timed out
TimeoutCancellationToken was created here:
#0 /usr/app/vendor/amphp/postgres/src/TimeoutConnector.php:39 Amp\TimeoutCancellationToken->__construct()
#1 /usr/app/vendor/amphp/postgres/src/functions.php:43 Amp\Postgres\TimeoutConnector->connect()
#2 /usr/app/run.php:13 Amp\Postgres\connect()
#3 {closure}()
#4 /usr/app/vendor/amphp/amp/lib/Coroutine.php:67 Generator->current()
#5 /usr/app/vendor/amphp/amp/lib/Loop/Driver.php:126 Amp\Coroutine->__construct()
#6 /usr/app/vendor/amphp/amp/lib/Loop/Driver.php:72 Amp\Loop\Driver->tick()
#7 /usr/app/vendor/amphp/amp/lib/Loop/EventDriver.php:209 Amp\Loop\Driver->run()
#8 /usr/app/vendor/amphp/amp/lib/Loop.php:95 Amp\Loop\EventDriver->run()
#9 /usr/app/run.php:22 Amp\Loop::run() in /usr/app/vendor/amphp/amp/lib/TimeoutCancellationToken.php:30
Stack trace:
#0 /usr/app/vendor/amphp/amp/lib/Loop/EventDriver.php(101): Amp\TimeoutCancellationToken::Amp\{closure}('b', NULL)
#1 [internal function]: Amp\Loop\EventDriver->Amp\ in /usr/app/vendor/amphp/amp/lib/CancellationTokenSource.php on line 161

‌

[Jáchym Toušek]

@Ruslan Osmanov I’m guessing that this makes the bug quite a bit harder to track down, right?

[Ruslan Osmanov]

@Jáchym Toušek , Well, yes. I’ll have to track down the place the stack overflow occurs. Maybe __stack_chk_fail gets deactivated with my debug build. I’ll need to analyze it through valgrind or something.

I think I have a terrible bug here:

            } else if (php_stream_can_cast(stream, PHP_STREAM_AS_STDIO | PHP_STREAM_CAST_INTERNAL) == SUCCESS) {
                if (php_stream_cast(stream, PHP_STREAM_AS_STDIO,
                            (void *) &file_desc, 1) != SUCCESS || file_desc < 0) {
                    return -1;
                }

The problem is that file_desc is a php_socket_t which is int, and php_stream_cast treats it &file_desc as a FILE** in this case (when PHP_STREAM_AS_STDIO flag is passed)

I think a patch similar to the following should fix the issue:

diff --git a/php7/src/util.c b/php7/src/util.c
index e7d1783..9b9ea0b 100644
--- a/php7/src/util.c
+++ b/php7/src/util.c
@@ -29,6 +29,7 @@ php_socket_t php_event_zval_to_fd(zval *pfd)
 {
        php_socket_t  file_desc = -1;
        php_stream   *stream;
+       FILE *fp;
 #ifdef PHP_EVENT_SOCKETS_SUPPORT
        php_socket   *php_sock;
 #endif
@@ -51,7 +52,12 @@ php_socket_t php_event_zval_to_fd(zval *pfd)
                        }

                        /* PHP stream */
-                       if (php_stream_can_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL) == SUCCESS) {
+                       if (php_stream_is(stream, PHP_STREAM_IS_STDIO)) {
+                               if (FAILURE == php_stream_cast(stream, PHP_STREAM_AS_STDIO, (void**)&fp, REPORT_ERRORS)) {
+                                       return -1;
+                               }
+                               file_desc = fileno(fp);
+                       } else if (php_stream_can_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL) == SUCCESS) {
                                if (php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL,
                                                        (void *) &file_desc, 1) != SUCCESS || file_desc < 0) {
                                        return -1;

Haven’t tested it yet. Unfortunately, I don’t have much free time. I’ll test it out as soon as I can.

Thanks.

[Jáchym Toušek]

@Ruslan Osmanov I understand. Thank you for looking into it even with your busy schedule!

[Ruslan Osmanov]

@Jáchym Toušek , so the resource passed to Event is a “PostgresQL link” which is not expected. The resource appears to refer to a kind of STDIO stream, but, apparently, needs special parsing.

I think I’ve fixed the segfault in version 2.5.6, but it won’t work until I add pgsql support. It doesn’t seem to be very difficult, but requires extra time. At a first glance, pgsql doesn’t seem to provide sufficient API for this task, but we could find a workaround. I’ll try to do this some time later.

[Jáchym Toušek]

As per trowski’s advice I changed my app to use ext-pq instead.

[Ruslan Osmanov]

I'll consider adding support for pgsql resource type in #62.