Skip to content

Commit f1fedec

Browse files
committed
Reset serialization state before request input processing
Serialization and unserialization contexts are stored in module globals, but their allocations belong to the current request heap. If a bailout skips the normal context cleanup, those globals can retain pointers to memory that is freed at request shutdown. The next request can process input before module RINIT runs. In particular, multipart upload-progress handling can invoke session serialization during input parsing, before the existing RINIT reset. It may therefore observe the previous request’s stale context and lock state. Resetting only in RINIT is too late to protect this path. Clear the serialization lock and both context structures at the beginning of request startup, before input processing. This discards the bookkeeping without following potentially invalid pointers or running destructors. Remove the redundant reset from basic-module RINIT. Add regression coverage that abandons serialize or unserialize state via a bailout, then sends a multipart upload-progress request to the same worker. Verify the follow-up request sees cleared state for locked and unlocked cases. Validation: the regression passes with the startup reset alone and fails when both resets are removed.
1 parent 9705fda commit f1fedec

11 files changed

Lines changed: 251 additions & 10 deletions

File tree

‎Zend/tests/arginfo_zpp_mismatch.inc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ function skipFunction($function): bool {
1010
|| $function === 'exit'
1111
|| $function === 'die'
1212
|| $function === 'zend_trigger_bailout'
13+
|| $function === 'zend_test_serialization_bailout'
1314
/* intentionally violate invariants */
1415
|| $function === 'zend_create_unterminated_string'
1516
|| $function === 'zend_test_array_return'

‎ext/standard/basic_functions.c‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -390,10 +390,6 @@ PHP_RINIT_FUNCTION(basic) /* {{{ */
390390
{
391391
memset(BG(strtok_table), 0, 256);
392392

393-
BG(serialize_lock) = 0;
394-
memset(&BG(serialize), 0, sizeof(BG(serialize)));
395-
memset(&BG(unserialize), 0, sizeof(BG(unserialize)));
396-
397393
BG(strtok_string) = NULL;
398394
BG(strtok_last) = NULL;
399395
BG(ctype_string) = NULL;

‎ext/standard/php_var.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ PHPAPI zend_result php_var_export_ex(zval *struc, int level, smart_str *buf);
2626

2727
PHPAPI void php_debug_zval_dump(zval *struc, int level);
2828

29+
/* Reset before request input processing, never during serialization. */
30+
PHPAPI void php_var_serialization_reset(void);
31+
2932
typedef struct php_serialize_data *php_serialize_data_t;
3033
typedef struct php_unserialize_data *php_unserialize_data_t;
3134

‎ext/standard/var.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,6 +1406,14 @@ PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t
14061406
}
14071407
/* }}} */
14081408

1409+
PHPAPI void php_var_serialization_reset(void)
1410+
{
1411+
/* A bailout can abandon contexts that the request heap will reclaim. */
1412+
BG(serialize_lock) = 0;
1413+
memset(&BG(serialize), 0, sizeof(BG(serialize)));
1414+
memset(&BG(unserialize), 0, sizeof(BG(unserialize)));
1415+
}
1416+
14091417
PHPAPI php_serialize_data_t php_var_serialize_init(void) {
14101418
struct php_serialize_data *d;
14111419
/* fprintf(stderr, "SERIALIZE_INIT == lock: %u, level: %u\n", BG(serialize_lock), BG(serialize).level); */

‎ext/zend_test/test.c‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "php.h"
2222
#include "php_ini.h"
2323
#include "ext/standard/info.h"
24+
#include "ext/standard/php_var.h"
2425
#include "php_test.h"
2526
#include "observer.h"
2627
#include "fiber.h"
@@ -200,6 +201,38 @@ static ZEND_FUNCTION(zend_leak_bytes)
200201
emalloc(leakbytes);
201202
}
202203

204+
static ZEND_FUNCTION(zend_test_serialization_bailout)
205+
{
206+
bool unserialize, locked;
207+
208+
ZEND_PARSE_PARAMETERS_START(2, 2)
209+
Z_PARAM_BOOL(unserialize)
210+
Z_PARAM_BOOL(locked)
211+
ZEND_PARSE_PARAMETERS_END();
212+
213+
if (unserialize) {
214+
(void) php_var_unserialize_init();
215+
} else {
216+
(void) php_var_serialize_init();
217+
}
218+
if (locked) {
219+
BG(serialize_lock)++;
220+
}
221+
zend_bailout();
222+
}
223+
224+
static ZEND_FUNCTION(zend_test_serialization_state)
225+
{
226+
ZEND_PARSE_PARAMETERS_NONE();
227+
228+
array_init(return_value);
229+
add_assoc_long(return_value, "lock", BG(serialize_lock));
230+
add_assoc_long(return_value, "serialize_level", BG(serialize).level);
231+
add_assoc_bool(return_value, "serialize_data", BG(serialize).data != NULL);
232+
add_assoc_long(return_value, "unserialize_level", BG(unserialize).level);
233+
add_assoc_bool(return_value, "unserialize_data", BG(unserialize).data != NULL);
234+
}
235+
203236
/* Reallocate a block of old_size bytes to new_size bytes and return its block
204237
* size, along with the block size of a fresh allocation of new_size bytes. */
205238
static ZEND_FUNCTION(zend_test_erealloc_block_size)

‎ext/zend_test/test.stub.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,10 @@ function zend_delref(mixed $variable): void {}
301301

302302
function zend_test_erealloc_block_size(int $old_size, int $new_size): array {}
303303

304+
function zend_test_serialization_bailout(bool $unserialize, bool $locked): never {}
305+
306+
function zend_test_serialization_state(): array {}
307+
304308
function zend_bool(bool $param): bool {}
305309
function zend_bool_or_null(bool|null $param): bool|null {}
306310
function zend_bool_slow_zpp(bool $param): bool {}

‎ext/zend_test/test_arginfo.h‎

Lines changed: 12 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/zend_test/test_decl.h‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/zend_test/test_legacy_arginfo.h‎

Lines changed: 12 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎main/main.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1854,6 +1854,8 @@ zend_result php_request_startup(void)
18541854
{
18551855
zend_result retval = SUCCESS;
18561856

1857+
/* Input processing can use serialization before module RINIT. */
1858+
php_var_serialization_reset();
18571859
zend_interned_strings_activate();
18581860

18591861
#ifdef HAVE_DTRACE

0 commit comments

Comments
 (0)