Skip to content

Commit f5501ea

Browse files
committed
fix: lazy-reseed GC random state per worker process
Replace the per-call php_random_bytes_silent() approach with lazy reseeding of PS(random) on first use. GINIT initializes the PCG state but runs before PHP-FPM forks workers, so all workers would otherwise inherit and replay the same sequence. A one-time CSPRNG call on first GC check per process breaks the correlation while keeping subsequent draws cheap (PCG, not CSPRNG). Add gc_random_seeded bool to php_ps_globals to track whether the per- process reseed has happened. Restore the random struct initialization in GINIT (removed in previous commit) since the field is now actively used again.
1 parent ef12ecd commit f5501ea

2 files changed

Lines changed: 22 additions & 9 deletions

File tree

ext/session/php_session.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,9 @@ typedef struct _php_ps_globals {
146146
zend_string *session_started_filename;
147147
uint32_t session_started_lineno;
148148
int module_number;
149-
/* Unused since the GC probability draw was made stateless; retained only
150-
* to preserve struct layout (ABI) on stable branches. */
151149
php_random_status_state_pcgoneseq128xslrr64 random_state;
152150
php_random_algo_with_state random;
151+
bool gc_random_seeded; /* false until first GC check; reseeds after fork */
153152
zend_long gc_probability;
154153
zend_long gc_divisor;
155154
zend_long gc_maxlifetime;

ext/session/session.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -393,14 +393,22 @@ static zend_long php_session_gc(bool immediate)
393393
/* GC must be done before reading session data. */
394394
if ((PS(mod_data) || PS(mod_user_implemented))) {
395395
if (!collect && PS(gc_probability) > 0) {
396-
/* Draw fresh entropy per request instead of using the per-process
397-
* PS(random) state. The latter is seeded once in GINIT, which runs
398-
* before SAPIs such as PHP-FPM fork their workers, so every worker
399-
* would inherit and replay the same GC-decision sequence. */
400-
uint32_t rand_val;
401-
if (php_random_bytes_silent(&rand_val, sizeof(rand_val)) == SUCCESS) {
402-
collect = (rand_val % (uint32_t) PS(gc_divisor)) < (uint32_t) PS(gc_probability);
396+
/* Reseed PS(random) on first use per process. GINIT runs in the
397+
* master process before SAPIs like PHP-FPM fork their workers, so
398+
* all workers would otherwise inherit and replay the same PCG
399+
* sequence. One CSPRNG call here breaks the correlation. */
400+
if (UNEXPECTED(!PS(gc_random_seeded))) {
401+
php_random_uint128_t seed;
402+
if (php_random_bytes_silent(&seed, sizeof(seed)) == FAILURE) {
403+
seed = php_random_uint128_constant(
404+
php_random_generate_fallback_seed(),
405+
php_random_generate_fallback_seed()
406+
);
407+
}
408+
php_random_pcgoneseq128xslrr64_seed128(PS(random).state, seed);
409+
PS(gc_random_seeded) = true;
403410
}
411+
collect = php_random_range(PS(random), 0, PS(gc_divisor) - 1) < PS(gc_probability);
404412
}
405413

406414
if (collect) {
@@ -2892,6 +2900,12 @@ static PHP_GINIT_FUNCTION(ps)
28922900
ZVAL_UNDEF(&ps_globals->mod_user_names.ps_validate_sid);
28932901
ZVAL_UNDEF(&ps_globals->mod_user_names.ps_update_timestamp);
28942902
ZVAL_UNDEF(&ps_globals->http_session_vars);
2903+
2904+
ps_globals->random = (php_random_algo_with_state){
2905+
.algo = &php_random_algo_pcgoneseq128xslrr64,
2906+
.state = &ps_globals->random_state,
2907+
};
2908+
ps_globals->gc_random_seeded = false;
28952909
}
28962910

28972911
static PHP_MINIT_FUNCTION(session)

0 commit comments

Comments
 (0)