From cb8990033a4f4ef4fa0c89a437a7927d45f819d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Fri, 1 Mar 2024 17:54:48 +0100 Subject: [PATCH 1/2] session: Slightly reorder the members within the module globals The previous ordering resulted in a needlessly large number of holes and split several `zval`s across cache line boundaries. Do the bare minimum of reordering to keep related members grouped, but reducing the struct size by 32 bytes and keeping `zval`s within a single cache line. Before: struct _php_session_rfc1867_progress { size_t sname_len; /* 0 8 */ zval sid; /* 8 16 */ smart_str key; /* 24 16 */ zend_long update_step; /* 40 8 */ zend_long next_update; /* 48 8 */ double next_update_time; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ _Bool cancel_upload; /* 64 1 */ _Bool apply_trans_sid; /* 65 1 */ /* XXX 6 bytes hole, try to pack */ size_t content_length; /* 72 8 */ zval data; /* 80 16 */ zval * post_bytes_processed; /* 96 8 */ zval files; /* 104 16 */ zval current_file; /* 120 16 */ /* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */ zval * current_file_bytes_processed; /* 136 8 */ /* size: 144, cachelines: 3, members: 14 */ /* sum members: 138, holes: 1, sum holes: 6 */ /* last cacheline: 16 bytes */ }; struct _php_ps_globals { char * save_path; /* 0 8 */ char * session_name; /* 8 8 */ zend_string * id; /* 16 8 */ char * extern_referer_chk; /* 24 8 */ char * cache_limiter; /* 32 8 */ zend_long cookie_lifetime; /* 40 8 */ char * cookie_path; /* 48 8 */ char * cookie_domain; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ _Bool cookie_secure; /* 64 1 */ _Bool cookie_httponly; /* 65 1 */ /* XXX 6 bytes hole, try to pack */ char * cookie_samesite; /* 72 8 */ const ps_module * mod; /* 80 8 */ const ps_module * default_mod; /* 88 8 */ void * mod_data; /* 96 8 */ php_session_status session_status; /* 104 4 */ /* XXX 4 bytes hole, try to pack */ zend_string * session_started_filename; /* 112 8 */ uint32_t session_started_lineno; /* 120 4 */ /* XXX 4 bytes hole, try to pack */ /* --- cacheline 2 boundary (128 bytes) --- */ zend_long gc_probability; /* 128 8 */ zend_long gc_divisor; /* 136 8 */ zend_long gc_maxlifetime; /* 144 8 */ int module_number; /* 152 4 */ /* XXX 4 bytes hole, try to pack */ zend_long cache_expire; /* 160 8 */ struct { zval ps_open; /* 168 16 */ zval ps_close; /* 184 16 */ /* --- cacheline 3 boundary (192 bytes) was 8 bytes ago --- */ zval ps_read; /* 200 16 */ zval ps_write; /* 216 16 */ zval ps_destroy; /* 232 16 */ zval ps_gc; /* 248 16 */ /* --- cacheline 4 boundary (256 bytes) was 8 bytes ago --- */ zval ps_create_sid; /* 264 16 */ zval ps_validate_sid; /* 280 16 */ zval ps_update_timestamp; /* 296 16 */ } mod_user_names; /* 168 144 */ _Bool mod_user_implemented; /* 312 1 */ _Bool mod_user_is_open; /* 313 1 */ /* XXX 6 bytes hole, try to pack */ /* --- cacheline 5 boundary (320 bytes) --- */ zend_string * mod_user_class_name; /* 320 8 */ const struct ps_serializer_struct * serializer; /* 328 8 */ zval http_session_vars; /* 336 16 */ _Bool auto_start; /* 352 1 */ _Bool use_cookies; /* 353 1 */ _Bool use_only_cookies; /* 354 1 */ _Bool use_trans_sid; /* 355 1 */ /* XXX 4 bytes hole, try to pack */ zend_long sid_length; /* 360 8 */ zend_long sid_bits_per_character; /* 368 8 */ _Bool send_cookie; /* 376 1 */ _Bool define_sid; /* 377 1 */ /* XXX 6 bytes hole, try to pack */ /* --- cacheline 6 boundary (384 bytes) --- */ php_session_rfc1867_progress * rfc1867_progress; /* 384 8 */ _Bool rfc1867_enabled; /* 392 1 */ _Bool rfc1867_cleanup; /* 393 1 */ /* XXX 6 bytes hole, try to pack */ char * rfc1867_prefix; /* 400 8 */ char * rfc1867_name; /* 408 8 */ zend_long rfc1867_freq; /* 416 8 */ double rfc1867_min_freq; /* 424 8 */ _Bool use_strict_mode; /* 432 1 */ _Bool lazy_write; /* 433 1 */ _Bool in_save_handler; /* 434 1 */ _Bool set_handler; /* 435 1 */ /* XXX 4 bytes hole, try to pack */ zend_string * session_vars; /* 440 8 */ /* size: 448, cachelines: 7, members: 48 */ /* sum members: 404, holes: 9, sum holes: 44 */ }; After: struct _php_session_rfc1867_progress { size_t sname_len; /* 0 8 */ zval sid; /* 8 16 */ smart_str key; /* 24 16 */ zend_long update_step; /* 40 8 */ zend_long next_update; /* 48 8 */ double next_update_time; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ _Bool cancel_upload; /* 64 1 */ _Bool apply_trans_sid; /* 65 1 */ /* XXX 6 bytes hole, try to pack */ size_t content_length; /* 72 8 */ zval data; /* 80 16 */ zval files; /* 96 16 */ zval * post_bytes_processed; /* 112 8 */ zval * current_file_bytes_processed; /* 120 8 */ /* --- cacheline 2 boundary (128 bytes) --- */ zval current_file; /* 128 16 */ /* size: 144, cachelines: 3, members: 14 */ /* sum members: 138, holes: 1, sum holes: 6 */ /* last cacheline: 16 bytes */ }; struct _php_ps_globals { char * save_path; /* 0 8 */ char * session_name; /* 8 8 */ zend_string * id; /* 16 8 */ char * extern_referer_chk; /* 24 8 */ char * cache_limiter; /* 32 8 */ zend_long cookie_lifetime; /* 40 8 */ char * cookie_path; /* 48 8 */ char * cookie_domain; /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ char * cookie_samesite; /* 64 8 */ _Bool cookie_secure; /* 72 1 */ _Bool cookie_httponly; /* 73 1 */ /* XXX 6 bytes hole, try to pack */ const ps_module * mod; /* 80 8 */ const ps_module * default_mod; /* 88 8 */ void * mod_data; /* 96 8 */ php_session_status session_status; /* 104 4 */ /* XXX 4 bytes hole, try to pack */ zend_string * session_started_filename; /* 112 8 */ uint32_t session_started_lineno; /* 120 4 */ int module_number; /* 124 4 */ /* --- cacheline 2 boundary (128 bytes) --- */ zend_long gc_probability; /* 128 8 */ zend_long gc_divisor; /* 136 8 */ zend_long gc_maxlifetime; /* 144 8 */ zend_long cache_expire; /* 152 8 */ struct { zval ps_open; /* 160 16 */ zval ps_close; /* 176 16 */ /* --- cacheline 3 boundary (192 bytes) --- */ zval ps_read; /* 192 16 */ zval ps_write; /* 208 16 */ zval ps_destroy; /* 224 16 */ zval ps_gc; /* 240 16 */ /* --- cacheline 4 boundary (256 bytes) --- */ zval ps_create_sid; /* 256 16 */ zval ps_validate_sid; /* 272 16 */ zval ps_update_timestamp; /* 288 16 */ } mod_user_names; /* 160 144 */ zend_string * mod_user_class_name; /* 304 8 */ _Bool mod_user_implemented; /* 312 1 */ _Bool mod_user_is_open; /* 313 1 */ _Bool auto_start; /* 314 1 */ _Bool use_cookies; /* 315 1 */ _Bool use_only_cookies; /* 316 1 */ _Bool use_trans_sid; /* 317 1 */ _Bool send_cookie; /* 318 1 */ _Bool define_sid; /* 319 1 */ /* --- cacheline 5 boundary (320 bytes) --- */ const struct ps_serializer_struct * serializer; /* 320 8 */ zval http_session_vars; /* 328 16 */ zend_long sid_length; /* 344 8 */ zend_long sid_bits_per_character; /* 352 8 */ php_session_rfc1867_progress * rfc1867_progress; /* 360 8 */ char * rfc1867_prefix; /* 368 8 */ char * rfc1867_name; /* 376 8 */ /* --- cacheline 6 boundary (384 bytes) --- */ zend_long rfc1867_freq; /* 384 8 */ double rfc1867_min_freq; /* 392 8 */ _Bool rfc1867_enabled; /* 400 1 */ _Bool rfc1867_cleanup; /* 401 1 */ _Bool use_strict_mode; /* 402 1 */ _Bool lazy_write; /* 403 1 */ _Bool in_save_handler; /* 404 1 */ _Bool set_handler; /* 405 1 */ /* XXX 2 bytes hole, try to pack */ zend_string * session_vars; /* 408 8 */ /* size: 416, cachelines: 7, members: 48 */ /* sum members: 404, holes: 3, sum holes: 12 */ /* last cacheline: 32 bytes */ }; --- ext/session/php_session.h | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/ext/session/php_session.h b/ext/session/php_session.h index 341aac5716ac2..1cb2b2276f6e0 100644 --- a/ext/session/php_session.h +++ b/ext/session/php_session.h @@ -132,10 +132,10 @@ typedef struct _php_session_rfc1867_progress { size_t content_length; zval data; /* the array exported to session data */ - zval *post_bytes_processed; /* data["bytes_processed"] */ zval files; /* data["files"] array */ - zval current_file; /* array of currently uploading file */ + zval *post_bytes_processed; /* data["bytes_processed"] */ zval *current_file_bytes_processed; + zval current_file; /* array of currently uploading file */ } php_session_rfc1867_progress; typedef struct _php_ps_globals { @@ -147,19 +147,19 @@ typedef struct _php_ps_globals { zend_long cookie_lifetime; char *cookie_path; char *cookie_domain; + char *cookie_samesite; bool cookie_secure; bool cookie_httponly; - char *cookie_samesite; const ps_module *mod; const ps_module *default_mod; void *mod_data; php_session_status session_status; zend_string *session_started_filename; uint32_t session_started_lineno; + int module_number; zend_long gc_probability; zend_long gc_divisor; zend_long gc_maxlifetime; - int module_number; zend_long cache_expire; struct { zval ps_open; @@ -172,28 +172,29 @@ typedef struct _php_ps_globals { zval ps_validate_sid; zval ps_update_timestamp; } mod_user_names; + zend_string *mod_user_class_name; bool mod_user_implemented; bool mod_user_is_open; - zend_string *mod_user_class_name; - const struct ps_serializer_struct *serializer; - zval http_session_vars; bool auto_start; bool use_cookies; bool use_only_cookies; bool use_trans_sid; /* contains the INI value of whether to use trans-sid */ + bool send_cookie; + bool define_sid; + + const struct ps_serializer_struct *serializer; + zval http_session_vars; zend_long sid_length; zend_long sid_bits_per_character; - bool send_cookie; - bool define_sid; php_session_rfc1867_progress *rfc1867_progress; - bool rfc1867_enabled; /* session.upload_progress.enabled */ - bool rfc1867_cleanup; /* session.upload_progress.cleanup */ char *rfc1867_prefix; /* session.upload_progress.prefix */ char *rfc1867_name; /* session.upload_progress.name */ zend_long rfc1867_freq; /* session.upload_progress.freq */ double rfc1867_min_freq; /* session.upload_progress.min_freq */ + bool rfc1867_enabled; /* session.upload_progress.enabled */ + bool rfc1867_cleanup; /* session.upload_progress.cleanup */ bool use_strict_mode; /* whether or not PHP accepts unknown session ids */ bool lazy_write; /* omit session write when it is possible */ From 4e14ea8824135845d1ec197450d642a6e109787b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Fri, 1 Mar 2024 18:15:48 +0100 Subject: [PATCH 2/2] session: Stop using php_combined_lcg() The CombinedLCG is a terrible RNG with a questionable API and should ideally not be used anymore. While in the case of ext/session it is only used for probabilistic garbage collection where the quality of the RNG is not of particular importance, there are better choices. Replace the RNG used for garbage collection by an ext/session specific instance of PcgOneseq128XslRr64. Its 16 Byte state nicely fits into the memory freed up by the previous reordering of the session globals struct, even allowing to the storage of the php_random_algo_with_state struct, making using the RNG a little nicer. Instead multiplying the float returned by the CombinedLCG by the GC Divisor to obtain an integer between 0 and the divisor we can just use `php_random_range` to directly generate an appropriate integer, completely avoiding the floating point maths, making it easier to verify the code for correctness. --- ext/session/php_session.h | 3 +++ ext/session/session.c | 21 +++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/ext/session/php_session.h b/ext/session/php_session.h index 1cb2b2276f6e0..c9611e6159709 100644 --- a/ext/session/php_session.h +++ b/ext/session/php_session.h @@ -19,6 +19,7 @@ #include "ext/standard/php_var.h" #include "ext/hash/php_hash.h" +#include "ext/random/php_random.h" #define PHP_SESSION_API 20161017 @@ -157,6 +158,8 @@ typedef struct _php_ps_globals { zend_string *session_started_filename; uint32_t session_started_lineno; int module_number; + php_random_status_state_pcgoneseq128xslrr64 random_state; + php_random_algo_with_state random; zend_long gc_probability; zend_long gc_divisor; zend_long gc_maxlifetime; diff --git a/ext/session/session.c b/ext/session/session.c index 61d91b77eaac1..ba4bb6619766c 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -387,17 +387,16 @@ PHPAPI zend_result php_session_valid_key(const char *key) /* {{{ */ static zend_long php_session_gc(bool immediate) /* {{{ */ { - int nrand; zend_long num = -1; + bool collect = immediate; /* GC must be done before reading session data. */ if ((PS(mod_data) || PS(mod_user_implemented))) { - if (immediate) { - PS(mod)->s_gc(&PS(mod_data), PS(gc_maxlifetime), &num); - return num; + if (!collect && PS(gc_probability) > 0) { + collect = php_random_range(PS(random), 0, PS(gc_divisor) - 1) < PS(gc_probability); } - nrand = (zend_long) ((float) PS(gc_divisor) * php_combined_lcg()); - if (PS(gc_probability) > 0 && nrand < PS(gc_probability)) { + + if (collect) { PS(mod)->s_gc(&PS(mod_data), PS(gc_maxlifetime), &num); } } @@ -2872,6 +2871,16 @@ static PHP_GINIT_FUNCTION(ps) /* {{{ */ ZVAL_UNDEF(&ps_globals->mod_user_names.ps_validate_sid); ZVAL_UNDEF(&ps_globals->mod_user_names.ps_update_timestamp); ZVAL_UNDEF(&ps_globals->http_session_vars); + + ps_globals->random = (php_random_algo_with_state){ + .algo = &php_random_algo_pcgoneseq128xslrr64, + .state = &ps_globals->random_state, + }; + php_random_uint128_t seed; + if (php_random_bytes_silent(&seed, sizeof(seed)) == FAILURE) { + seed = php_random_uint128_constant(GENERATE_SEED(), GENERATE_SEED()); + } + php_random_pcgoneseq128xslrr64_seed128(ps_globals->random.state, seed); } /* }}} */