- changed title to Segmentation fault on alpine
Segmentation fault on alpine
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 ()
Comments (24)
-
reporter -
repo owner - changed status to on hold
@Jáchym Toušek, could you add the steps to reproduce?
-
reporter @Ruslan Osmanov Unfortunately not since it happens with proprietary code.
-
reporter 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.
-
repo owner 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.
-
reporter Alright, I’ll try to create a reproducer then. Thanks!
-
reporter 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
-
repo owner - changed status to open
-
reporter Any news on this?
-
repo owner 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()
) beforeEvent::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 withevent_new()
.Event::timer
andEvent::signal
do similar checks and returnfalse
in case of an error. So I can’t see howEvent::add
could be called before the Event object is constructed properly (race condition? )Maybe you have ideas.
-
repo owner 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, ifEvent::free
is not called beforeEvent::add
-
reporter 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
-
repo owner Uploaded version 2.5.5 to PECL.
-
reporter @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.
-
reporter Ah, okay, lemme try.
-
reporter @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
-
repo owner Oh, sorry. That’s yet another one. I’ve actually fixed another bug. Okay.
-
repo owner 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://osmanov@bitbucket.org/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
-
reporter @Ruslan Osmanov I’m guessing that this makes the bug quite a bit harder to track down, right?
-
repo owner @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 aphp_socket_t
which isint
, andphp_stream_cast
treats it&file_desc
as aFILE**
in this case (whenPHP_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.
-
reporter @Ruslan Osmanov I understand. Thank you for looking into it even with your busy schedule!
-
repo owner @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.
-
reporter As per trowski’s advice I changed my app to use ext-pq instead.
-
repo owner - changed status to resolved
I'll consider adding support for pgsql resource type in #62.
- Log in to comment