Bug #65667 [Com]: ftp_nb_continue produces segfault

Edit report at https://bugs.php.net/bug.php?id=65667&edit=1 ID: 65667 Comment by: Terry at ellisons dot org dot uk Reported by: imprec at gmail dot com Summary: ftp_nb_continue produces segfault Status: Closed Type: Bug Package: FTP related Operating System: OSX PHP Version: 5.5.3 Block user comment: N Private report: N New Comment: The fix posted as https://github.com/php/php-src/commit/96cc419924c38874f9e2f2e5ccf3cd0430d90f43 stops the segfault, but doesn't fix the underlying bug. This can be seen by running ext/ftp/tests/ftp_nb_get_large.phpt [Sun Dec 29 19:05:11 2013] Script: '.../ftp_nb_get_large.php' .../main/streams/streams.c(292) : Freeing 0x7FF7EB20F9C0 (248 bytes), script=.../ext/ftp/tests/ftp_nb_get_large.php .../ext/ftp/php_ftp.c(938) : Actual location (location was relayed) === Total 1 memory leaks detected The output stream is stored in ftp->stream by ftp_nb_get() called from PHP_FUNCTION(ftp_nb_get) at ext/ftp/php_ftp.c:964. On the path ret == PHP_FTP_FINISHED the stream is then explicitly closed, but in the case where ret == PHP_FTP_MOREDATA it is now left open to allow further writes by subsequent PHP_FUNCTION(ftp_nb_continue) calls. Explicitly closing the stream is a botch after it has been assigned to the ftp resource; it should be closed by the ftp DTOR ftp_destructor_ftpbuf(), that is ftp.c:ftp_close(ftpbuf_t *ftp) should include the test if (ftp->stream) { php_stream_close(ftp->stream); ftp->stream = NULL; } and any early closes of the steam, eg. ftp.c:900, 906, 965, 972, 1005, 1157, 1161, 1216 should either also set ftp->stream to NULL or leave the close to the DTOR. Previous Comments: ------------------------------------------------------------------------ [2013-10-04 15:28:06] nikic@php.net Automatic comment on behalf of phofstetter@sensational.ch Revision: http://git.php.net/?p=php-src.git;a=commit;h=96cc419924c38874f9e2f2e5ccf3cd0430d90f43 Log: Fix bug #65667: ftp_nb_continue produces segfault ------------------------------------------------------------------------ [2013-10-02 06:40:03] phofstetter at sensational dot ch Ok. Official Pull-Request submitted here: https://github.com/php/php-src/pull/478 Sorry for the spam. I initially just wanted to confirm the issue, but then I felt compelled to dig deeper and deeper, commenting more and more :-) ------------------------------------------------------------------------ [2013-10-02 06:30:51] phofstetter at sensational dot ch I think the bug was introduced in https://github.com/php/php-src/commit/a93a462dcefd62e07963dd2da506fbb3409c88b5 where php_stream_close(outstream); is called unconditionally. Thus any further ftp_nb_continue() will work on a stream that has already been closed. When I restore the behaviour pre-patch, the segfault goes away. I will create a proper pull request and attach it to this bug. ------------------------------------------------------------------------ [2013-10-02 06:14:18] phofstetter at sensational dot ch and here's one stack frame higher (giving you the data you requested): (gdb) p ftp $1 = (ftpbuf_t *) 0x7ffff7fcf1f8 (gdb) p ftp->stream $2 = (php_stream *) 0x7ffff7fceb78 (gdb) p data $3 = (databuf_t *) 0x7ffff7fd1388 (gdb) p ftp->stream->ops $4 = (php_stream_ops *) 0x0 Again, something is wrong with that stream. ------------------------------------------------------------------------ [2013-10-02 06:00:46] phofstetter at sensational dot ch Here's a bit of poking around in gdb: Program received signal SIGSEGV, Segmentation fault. 0x000000000070080d in _php_stream_write (stream=0x18eecb8, buf=0x19511b4 "\243\060\060\060\060\060\060\060\061\243\r\nPAD\243\060\243\060\060\062\063\071\060\243\060\060\067\065\063\061\243\060\060\060\060\060\060\060\061\243\r\nPAD\243\060\243\060\060\062\063\071\060\243\060\060\067\063\066\061\243\060\060\060\060\060\060\060\062\243\r\nPAD\243\060\243\060\060\062\063\071\060\243\060\060\064\065\070\060\243\060\060\060\060\060\060\062\071\243\r\nPAD\243\060\243\060\060\062\063\071\060\243\060\060\067\060\060\066\243\060\060\060\060\060\060\060\063\243\r\nPAD\243\060\243\060\060\062\063\071\060\243\060\060\060\063\061\061\243\060\060\060\060\060\060\060\065\243\r\nPAD\243\060\243\060\060\062\063\071\060\243\060\060\066\063\061\065\243\060\060\060\060\060\060\060\066\243\r\nPA"..., count=1352) at /home/crazyhat/popscan-deb/downloads/php-5.5.4/main/streams/streams.c:1233 warning: Source file is more recent than executable. 1233 if (buf == NULL || count == 0 || stream->ops->write == NULL) { (gdb) p count $1 = 1352 (gdb) p stream $2 = (php_stream *) 0x18eecb8 (gdb) p stream->ops $3 = (php_stream_ops *) 0x0 (gdb) stream->ops seems to be NULL ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=65667 -- Edit this bug report at https://bugs.php.net/bug.php?id=65667&edit=1