From 5161cebe28cca36fa7f7989b5a799290a3f1eb6a Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 16 Jul 2019 13:27:41 +0200 Subject: [PATCH] Fix bug #52752 by not using mmap() to lex files Using mmap() is unsafe under concurrent modification. If the file is truncated, access past the end of the file may occur, which will generate a SIGBUS error. Even if the length does not change, the contents may, which is a situation that the lexer certainly is not prepared to deal with either. Reproduce with test.php: BBB' . "\r\n"); require_once __DIR__ . '/test.tpl'; And: for ((n=0;n<100;n++)); do sapi/cli/php test.php & done --- NEWS | 1 + Zend/zend_stream.c | 58 ---------------------------------------------- Zend/zend_stream.h | 2 -- main/main.c | 52 ++--------------------------------------- 4 files changed, 3 insertions(+), 110 deletions(-) diff --git a/NEWS b/NEWS index eedd6bc5914..adcfe8faa76 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,7 @@ PHP NEWS (Nikita) . Fixed bug #78066 (PHP eats the first byte of a program that comes from process substitution). (Nikita) + . Fixed bug #52752 (Crash when lexing). (Nikita) - Libxml: . Fixed bug #78279 (libxml_disable_entity_loader settings is shared between diff --git a/Zend/zend_stream.c b/Zend/zend_stream.c index 4b32203cbb7..87ed617ef05 100644 --- a/Zend/zend_stream.c +++ b/Zend/zend_stream.c @@ -23,27 +23,6 @@ #include "zend_compile.h" #include "zend_stream.h" -#if HAVE_MMAP -# if HAVE_UNISTD_H -# include -# if defined(_SC_PAGESIZE) -# define REAL_PAGE_SIZE sysconf(_SC_PAGESIZE); -# elif defined(_SC_PAGE_SIZE) -# define REAL_PAGE_SIZE sysconf(_SC_PAGE_SIZE); -# endif -# endif -# if HAVE_SYS_MMAN_H -# include -# endif -# ifndef REAL_PAGE_SIZE -# ifdef PAGE_SIZE -# define REAL_PAGE_SIZE PAGE_SIZE -# else -# define REAL_PAGE_SIZE 4096 -# endif -# endif -#endif - ZEND_DLIMPORT int isatty(int fd); static size_t zend_stream_stdio_reader(void *handle, char *buf, size_t len) /* {{{ */ @@ -73,17 +52,10 @@ static size_t zend_stream_stdio_fsizer(void *handle) /* {{{ */ } /* }}} */ static void zend_stream_unmap(zend_stream *stream) { /* {{{ */ -#if HAVE_MMAP - if (stream->mmap.map) { - munmap(stream->mmap.map, stream->mmap.len + ZEND_MMAP_AHEAD); - } else -#endif if (stream->mmap.buf) { efree(stream->mmap.buf); } stream->mmap.len = 0; - stream->mmap.pos = 0; - stream->mmap.map = 0; stream->mmap.buf = 0; stream->handle = stream->mmap.old_handle; } /* }}} */ @@ -197,7 +169,6 @@ ZEND_API int zend_stream_fixup(zend_file_handle *file_handle, char **buf, size_t break; case ZEND_HANDLE_MAPPED: - file_handle->handle.stream.mmap.pos = 0; *buf = file_handle->handle.stream.mmap.buf; *len = file_handle->handle.stream.mmap.len; return SUCCESS; @@ -215,30 +186,6 @@ ZEND_API int zend_stream_fixup(zend_file_handle *file_handle, char **buf, size_t file_handle->type = ZEND_HANDLE_STREAM; /* we might still be _FP but we need fsize() work */ if (old_type == ZEND_HANDLE_FP && !file_handle->handle.stream.isatty && size) { -#if HAVE_MMAP - size_t page_size = REAL_PAGE_SIZE; - - if (file_handle->handle.fp && - size != 0 && - ((size - 1) % page_size) <= page_size - ZEND_MMAP_AHEAD) { - /* *buf[size] is zeroed automatically by the kernel */ - *buf = mmap(0, size + ZEND_MMAP_AHEAD, PROT_READ, MAP_PRIVATE, fileno(file_handle->handle.fp), 0); - if (*buf != MAP_FAILED) { - zend_long offset = ftell(file_handle->handle.fp); - file_handle->handle.stream.mmap.map = *buf; - - if (offset != -1) { - *buf += offset; - size -= offset; - } - file_handle->handle.stream.mmap.buf = *buf; - file_handle->handle.stream.mmap.len = size; - - goto return_mapped; - } - } -#endif - file_handle->handle.stream.mmap.map = 0; file_handle->handle.stream.mmap.buf = *buf = safe_emalloc(1, size, ZEND_MMAP_AHEAD); file_handle->handle.stream.mmap.len = zend_stream_read(file_handle, *buf, size); } else { @@ -255,7 +202,6 @@ ZEND_API int zend_stream_fixup(zend_file_handle *file_handle, char **buf, size_t remain = size; } } - file_handle->handle.stream.mmap.map = 0; file_handle->handle.stream.mmap.len = size; if (size && remain < ZEND_MMAP_AHEAD) { *buf = safe_erealloc(*buf, size, 1, ZEND_MMAP_AHEAD); @@ -271,11 +217,7 @@ ZEND_API int zend_stream_fixup(zend_file_handle *file_handle, char **buf, size_t if (ZEND_MMAP_AHEAD) { memset(file_handle->handle.stream.mmap.buf + file_handle->handle.stream.mmap.len, 0, ZEND_MMAP_AHEAD); } -#if HAVE_MMAP -return_mapped: -#endif file_handle->type = ZEND_HANDLE_MAPPED; - file_handle->handle.stream.mmap.pos = 0; file_handle->handle.stream.mmap.old_handle = file_handle->handle.stream.handle; file_handle->handle.stream.mmap.old_closer = file_handle->handle.stream.closer; file_handle->handle.stream.handle = &file_handle->handle.stream; diff --git a/Zend/zend_stream.h b/Zend/zend_stream.h index c98969a3b4f..cf6d76437b3 100644 --- a/Zend/zend_stream.h +++ b/Zend/zend_stream.h @@ -44,8 +44,6 @@ typedef enum { typedef struct _zend_mmap { size_t len; - size_t pos; - void *map; char *buf; void *old_handle; zend_stream_closer_t old_closer; diff --git a/main/main.c b/main/main.c index 66cf8df419d..c9d37ac2e00 100644 --- a/main/main.c +++ b/main/main.c @@ -84,27 +84,6 @@ #include "rfc1867.h" #include "ext/standard/html_tables.h" - -#if HAVE_MMAP || defined(PHP_WIN32) -# if HAVE_UNISTD_H -# include -# if defined(_SC_PAGESIZE) -# define REAL_PAGE_SIZE sysconf(_SC_PAGESIZE); -# elif defined(_SC_PAGE_SIZE) -# define REAL_PAGE_SIZE sysconf(_SC_PAGE_SIZE); -# endif -# endif -# if HAVE_SYS_MMAN_H -# include -# endif -# ifndef REAL_PAGE_SIZE -# ifdef PAGE_SIZE -# define REAL_PAGE_SIZE PAGE_SIZE -# else -# define REAL_PAGE_SIZE 4096 -# endif -# endif -#endif /* }}} */ PHPAPI int (*php_register_internal_extensions_func)(void) = php_register_internal_extensions; @@ -1583,13 +1562,6 @@ static void php_zend_stream_closer(void *handle) /* {{{ */ } /* }}} */ -static void php_zend_stream_mmap_closer(void *handle) /* {{{ */ -{ - php_stream_mmap_unmap((php_stream*)handle); - php_zend_stream_closer(handle); -} -/* }}} */ - static size_t php_zend_stream_fsizer(void *handle) /* {{{ */ { php_stream_statbuf ssb; @@ -1608,38 +1580,18 @@ static int php_stream_open_for_zend(const char *filename, zend_file_handle *hand PHPAPI int php_stream_open_for_zend_ex(const char *filename, zend_file_handle *handle, int mode) /* {{{ */ { - char *p; - size_t len, mapped_len; php_stream *stream = php_stream_open_wrapper((char *)filename, "rb", mode, &handle->opened_path); if (stream) { -#if HAVE_MMAP || defined(PHP_WIN32) - size_t page_size = REAL_PAGE_SIZE; -#endif - + handle->type = ZEND_HANDLE_STREAM; handle->filename = (char*)filename; handle->free_filename = 0; handle->handle.stream.handle = stream; handle->handle.stream.reader = (zend_stream_reader_t)_php_stream_read; handle->handle.stream.fsizer = php_zend_stream_fsizer; handle->handle.stream.isatty = 0; - /* can we mmap immediately? */ memset(&handle->handle.stream.mmap, 0, sizeof(handle->handle.stream.mmap)); - len = php_zend_stream_fsizer(stream); - if (len != 0 -#if HAVE_MMAP || defined(PHP_WIN32) - && ((len - 1) % page_size) <= page_size - ZEND_MMAP_AHEAD -#endif - && php_stream_mmap_possible(stream) - && (p = php_stream_mmap_range(stream, 0, len, PHP_STREAM_MAP_MODE_SHARED_READONLY, &mapped_len)) != NULL) { - handle->handle.stream.closer = php_zend_stream_mmap_closer; - handle->handle.stream.mmap.buf = p; - handle->handle.stream.mmap.len = mapped_len; - handle->type = ZEND_HANDLE_MAPPED; - } else { - handle->handle.stream.closer = php_zend_stream_closer; - handle->type = ZEND_HANDLE_STREAM; - } + handle->handle.stream.closer = php_zend_stream_closer; /* suppress warning if this stream is not explicitly closed */ php_stream_auto_cleanup(stream);