The negative fiber.stack_size setting leads to crash

[dstogov]

Description

The problem is found by oss-fuzz #63715

The following code:

<?php
ini_set("fiber.stack_size","-1");
$fiber = new Fiber(function() {});
try {
    $fiber->start();
} catch (Throwable $e) {
	echo "Exception: " . $e->getMessage()."\n";
}
?>

Resulted in this output:

==557311==ERROR: AddressSanitizer: SEGV on unknown address 0x7ffbbe8e6fe8 (pc 0x000001c18aeb bp 0x7ffe3d8e4050 sp 0x7ffe3d8e4008 T0)
==557311==The signal is caused by a WRITE memory access.
    #0 0x1c18aeb in make_fcontext (/home/dmitry/php/php8.1/CGI-DEBUG-64/sapi/cli/php+0x1c18aeb)
    #1 0x236c940 in zim_Fiber_start /home/dmitry/php/php8.1/Zend/zend_fibers.c:710
    #2 0x1f94fbb in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER /home/dmitry/php/php8.1/Zend/zend_vm_execute.h:1761
    #3 0x221bd6d in execute_ex /home/dmitry/php/php8.1/Zend/zend_vm_execute.h:55820
    #4 0x223d044 in zend_execute /home/dmitry/php/php8.1/Zend/zend_vm_execute.h:60164
    #5 0x1e87ddf in zend_execute_scripts /home/dmitry/php/php8.1/Zend/zend.c:1852
    #6 0x1c2983b in php_execute_script /home/dmitry/php/php8.1/main/main.c:2542
    #7 0x25ea0c2 in do_cli /home/dmitry/php/php8.1/sapi/cli/php_cli.c:965
    #8 0x25ed1d2 in main /home/dmitry/php/php8.1/sapi/cli/php_cli.c:1367
    #9 0x7ffbc0e4a54f in __libc_start_call_main (/usr/lib64/../lib64/libc.so.6+0x2754f)

But I expected this output instead:

Exception: Fiber stack size is too small, it needs to be at least 8192 bytes

I would propose the following patch

diff --git a/Zend/zend.c b/Zend/zend.c
index 351e9ac5eea..e6f92410c08 100644
--- a/Zend/zend.c
+++ b/Zend/zend.c
@@ -177,7 +177,12 @@ static ZEND_INI_MH(OnSetExceptionStringParamMaxLen) /* {{{ */
 static ZEND_INI_MH(OnUpdateFiberStackSize) /* {{{ */
 {
 	if (new_value) {
-		EG(fiber_stack_size) = zend_atol(ZSTR_VAL(new_value), ZSTR_LEN(new_value));
+		zend_long tmp = zend_atol(ZSTR_VAL(new_value), ZSTR_LEN(new_value));
+		if (tmp < 0) {
+			zend_error(E_WARNING, "fiber.stack_size must be a positive number");
+			return FAILURE;
+		}
+		EG(fiber_stack_size) = tmp;
 	} else {
 		EG(fiber_stack_size) = ZEND_FIBER_DEFAULT_C_STACK_SIZE;
 	}

PHP Version

PHP-8.1

Operating System

[dstogov]

@trowski @iluuu1994 @nielsdos do you think the proposed fix is good enough?

[nielsdos]

@dstogov I think so

[iluuu1994]

@dstogov That change looks reasonable to me as well.

[trowski]

@dstogov LGTM, thanks for fixing this.