Disallow concurrent access to stream via signal handlers

Author: arnaud-lb

ext/standard/tests/streams/concurrent_access.phpt

@@ -0,0 +1,60 @@
+--TEST--
+Stream cannot be accessed concurrently via signal handlers
+--EXTENSIONS--
+pcntl
+posix
+--SKIPIF--
+<?php
+$pair = @stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);
+if (!$pair) die("skip stream_socket_pair() not available");
+if (!function_exists('pcntl_fork')) die("skip pcntl_fork() not available");
+?>
+--FILE--
+<?php
+
+[$read, $write] = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);
+stream_set_blocking($read, true);
+stream_set_timeout($read, 3600);
+
+pcntl_signal(SIGUSR1, function (int $signo) use ($read): void {
+    echo "handler\n";
+    try {
+        fread($read, 1);
+    } catch (Error $e) {
+        echo $e::class, ": ", $e->getMessage(), "\n";
+    }
+    try {
+        fclose($read);
+    } catch (Error $e) {
+        echo $e::class, ": ", $e->getMessage(), "\n";
+    }
+}, restart_syscalls: true);
+
+$pid = pcntl_fork();
+if ($pid === 0) {
+    fwrite($write, "1");
+    fclose($read);
+    usleep(200_000); // let parent block in fread()
+    posix_kill(posix_getppid(), SIGUSR1);
+    usleep(200_000); // let parent block in fread()
+    fwrite($write, "hello\n");
+    fclose($write);
+    exit(0);
+}
+
+// Wait for child to start
+fread($read, 1);
+
+$result = fread($read, 1024);
+var_dump($result);
+
+pcntl_waitpid($pid, $status);
+fclose($read);
+fclose($write);
+?>
+--EXPECT--
+handler
+Error: Concurrent access to a stream
+Error: Concurrent access to a stream
+string(6) "hello
+"

ext/standard/tests/streams/concurrent_access_fatal.phpt

@@ -0,0 +1,54 @@
+--TEST--
+Stream is closed while marked in_use after a fatal error
+--EXTENSIONS--
+pcntl
+posix
+--SKIPIF--
+<?php
+$pair = @stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);
+if (!$pair) die("skip stream_socket_pair() not available");
+if (!function_exists('pcntl_fork')) die("skip pcntl_fork() not available");
+?>
+--FILE--
+<?php
+
+[$read, $write] = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);
+stream_set_blocking($read, true);
+stream_set_timeout($read, 3600);
+
+$pid = pcntl_fork();
+if ($pid === 0) {
+    fwrite($write, "1");
+    fclose($read);
+    usleep(200_000); // let parent block in fread()
+    posix_kill(posix_getppid(), SIGUSR1);
+    exit(0);
+}
+
+pcntl_signal(SIGUSR1, function (int $signo) use ($read): void {
+    echo "handler\n";
+    eval('class bool {}');
+}, restart_syscalls: true);
+
+register_shutdown_function(function() use ($read) {
+    try {
+        fclose($read);
+    } catch (Error $e) {
+        echo "shutdown function still cannot access stream: ", $e::class, ": ", $e->getMessage(), "\n";
+        return;
+    }
+    echo "unreachable (shutdown function)\n";
+});
+
+// Wait for child to start
+fread($read, 1);
+
+$result = fread($read, 1024);
+
+echo "unreachable";
+?>
+--EXPECTF--
+handler
+
+Fatal error: Cannot use "bool" as a class name as it is reserved in %s : eval()'d code on line 1
+shutdown function still cannot access stream: Error: Concurrent access to a stream

main/php_streams.h

@@ -189,6 +189,11 @@ struct _php_stream_wrapper	{
 
 #define PHP_STREAM_FLAG_NO_IO						0x400
 
+/* Stream is in-use: accessing it would be unsafe. This is set when a stream
+ * operation is interrupted by a signal, before calling signal handlers.
+ * Stream ops and public stream APIs check this flag before proceeding. */
+#define PHP_STREAM_FLAG_IN_USE						0x800
+
 #define PHP_STREAM_FLAG_WAS_WRITTEN					0x80000000
 
 struct _php_stream  {
@@ -681,6 +686,32 @@ static inline bool php_is_stream_path(const char *filename)
 	return ((p != filename) && (p[0] == ':') && (p[1] == '/') && (p[2] == '/'));
 }
 
+static inline zend_signal_interrupt_result php_stream_check_signals(php_stream *stream)
+{
+	if (stream != NULL && zend_signal_interrupt_function != zend_signal_interrupt) {
+		uint32_t orig_in_use = stream->flags & PHP_STREAM_FLAG_IN_USE;
+		stream->flags |= PHP_STREAM_FLAG_IN_USE;
+		zend_signal_interrupt_result result = zend_signal_interrupt_function();
+		stream->flags &= ~PHP_STREAM_FLAG_IN_USE;
+		stream->flags |= orig_in_use;
+		return result;
+	}
+
+	return zend_signal_interrupt_function();
+}
+
+PHPAPI ZEND_COLD void php_stream_in_use_error(void);
+
+/* See PHP_STREAM_FLAG_IN_USE */
+static inline zend_result php_stream_check_in_use(php_stream *stream)
+{
+	if (UNEXPECTED(stream->flags & PHP_STREAM_FLAG_IN_USE)) {
+		php_stream_in_use_error();
+		return FAILURE;
+	}
+	return SUCCESS;
+}
+
 END_EXTERN_C()
 #endif
 

main/streams/streams.c

@@ -257,6 +257,12 @@ PHPAPI int _php_stream_free(php_stream *stream, int close_options) /* {{{ */
 	int release_cast = 1;
 	php_stream_context *context;
 
+	/* Not using php_stream_check_in_use() as we want to allow releasing file descriptors during resource shutdown */
+	if (UNEXPECTED((stream->flags & PHP_STREAM_FLAG_IN_USE) && !(EG(flags) & EG_FLAGS_IN_RESOURCE_SHUTDOWN))) {
+		php_stream_in_use_error();
+		return 0;
+	}
+
 	/* During shutdown resources may be released before other resources still holding them.
 	 * When only resources are referenced this is not a problem, because they are refcounted
 	 * and will only be fully freed once the refcount drops to zero. However, if php_stream*
@@ -435,6 +441,10 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
 {
 	/* allocate/fill the buffer */
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return FAILURE;
+	}
+
 	zend_result retval;
 	bool old_eof = stream->eof;
 
@@ -607,6 +617,10 @@ PHPAPI ssize_t _php_stream_read(php_stream *stream, char *buf, size_t size)
 {
 	ssize_t toread = 0, didread = 0;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return 0;
+	}
+
 	while (size > 0) {
 
 		/* take from the read buffer first.
@@ -709,6 +723,10 @@ PHPAPI zend_string *php_stream_read_to_str(php_stream *stream, size_t len)
 
 PHPAPI bool _php_stream_eof(php_stream *stream)
 {
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return 0;
+	}
+
 	/* if there is data in the buffer, it's not EOF */
 	if (stream->writepos - stream->readpos > 0) {
 		return 0;
@@ -758,6 +776,10 @@ PHPAPI bool _php_stream_puts(php_stream *stream, const char *buf)
 
 PHPAPI int _php_stream_stat(php_stream *stream, php_stream_statbuf *ssb)
 {
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return -1;
+	}
+
 	memset(ssb, 0, sizeof(*ssb));
 
 	/* if the stream was wrapped, allow the wrapper to stat it */
@@ -782,6 +804,10 @@ PHPAPI const char *php_stream_locate_eol(php_stream *stream, zend_string *buf)
 	const char *eol = NULL;
 	const char *readptr;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return 0;
+	}
+
 	if (!buf) {
 		readptr = (char*)stream->readbuf + stream->readpos;
 		avail = stream->writepos - stream->readpos;
@@ -827,6 +853,10 @@ PHPAPI char *_php_stream_get_line(php_stream *stream, char *buf, size_t maxlen,
 	int grow_mode = 0;
 	char *bufstart = buf;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return NULL;
+	}
+
 	if (buf == NULL) {
 		grow_mode = 1;
 	} else if (maxlen == 0) {
@@ -973,6 +1003,10 @@ PHPAPI zend_string *php_stream_get_record(php_stream *stream, size_t maxlen, con
 			tent_ret_len;			/* tentative returned length */
 	bool	has_delim = delim_len > 0;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return NULL;
+	}
+
 	if (maxlen == 0) {
 		return NULL;
 	}
@@ -1187,6 +1221,10 @@ PHPAPI int _php_stream_flush(php_stream *stream, int closing)
 {
 	int ret = 0;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return -1;
+	}
+
 	if (stream->writefilters.head && stream->ops->write) {
 		_php_stream_write_filtered(stream, NULL, 0, closing ? PSFS_FLAG_FLUSH_CLOSE : PSFS_FLAG_FLUSH_INC );
 	}
@@ -1204,6 +1242,10 @@ PHPAPI ssize_t _php_stream_write(php_stream *stream, const char *buf, size_t cou
 {
 	ssize_t bytes;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return (ssize_t) -1;
+	}
+
 	if (count == 0) {
 		return 0;
 	}
@@ -1306,6 +1348,10 @@ static zend_result php_stream_filters_seek_all(php_stream *stream, bool is_start
 
 PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence)
 {
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return -1;
+	}
+
 	if (stream->fclose_stdiocast == PHP_STREAM_FCLOSE_FOPENCOOKIE) {
 		/* flush can call seek internally so we need to prevent an infinite loop */
 		if (!stream->fclose_stdiocast_flush_in_progress) {
@@ -1415,6 +1461,10 @@ PHPAPI int _php_stream_set_option(php_stream *stream, int option, int value, voi
 {
 	int ret = PHP_STREAM_OPTION_RETURN_NOTIMPL;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return PHP_STREAM_OPTION_RETURN_ERR;
+	}
+
 	if (stream->ops->set_option) {
 		ret = stream->ops->set_option(stream, option, value, ptrparam);
 	}
@@ -1465,6 +1515,10 @@ PHPAPI ssize_t _php_stream_passthru(php_stream * stream STREAMS_DC)
 	char buf[8192];
 	ssize_t b;
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return -1;
+	}
+
 	if (php_stream_mmap_possible(stream)) {
 		char *p;
 		size_t mapped;
@@ -1506,6 +1560,10 @@ PHPAPI zend_string *_php_stream_copy_to_mem(php_stream *src, size_t maxlen, bool
 	php_stream_statbuf ssbuf;
 	zend_string *result;
 
+	if (php_stream_check_in_use(src) != SUCCESS) {
+		return NULL;
+	}
+
 	if (maxlen == 0) {
 		return ZSTR_EMPTY_ALLOC();
 	}
@@ -1594,7 +1652,6 @@ static zend_result php_stream_copy_fallback(php_stream *src, php_stream *dest, s
 {
 	char buf[CHUNK_SIZE];
 	size_t haveread = 0;
-
 	if (maxlen == PHP_STREAM_COPY_ALL) {
 		maxlen = 0;
 	}
@@ -2240,6 +2297,10 @@ PHPAPI php_stream_context *php_stream_context_set(php_stream *stream, php_stream
 {
 	php_stream_context *oldcontext = PHP_STREAM_CONTEXT(stream);
 
+	if (php_stream_check_in_use(stream) != SUCCESS) {
+		return NULL;
+	}
+
 	if (context) {
 		stream->ctx = context->res;
 		GC_ADDREF(context->res);
@@ -2430,3 +2491,8 @@ PHPAPI int _php_stream_scandir(const char *dirname, zend_string **namelist[], in
 	return -1;
 }
 /* }}} */
+
+PHPAPI ZEND_COLD void php_stream_in_use_error(void)
+{
+	zend_throw_error(NULL, "Concurrent access to a stream");
+}