From 8db6bd51a6b417ecbd374cbc4483dce9a4dea4d4 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Sun, 13 Nov 2022 15:54:08 -0500 Subject: [PATCH 1/3] Fix the replacement for shmget in Windows Closes #9944 https://man7.org/linux/man-pages/man2/shmget.2.html notes The name choice IPC_PRIVATE was perhaps unfortunate, IPC_NEW would more clearly show its function. --- TSRM/tsrm_win32.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/TSRM/tsrm_win32.c b/TSRM/tsrm_win32.c index a748a3b105f2e..9a0456c94806a 100644 --- a/TSRM/tsrm_win32.c +++ b/TSRM/tsrm_win32.c @@ -626,6 +626,9 @@ TSRM_API int shmget(key_t key, size_t size, int flags) shm_handle = OpenFileMapping(FILE_MAP_ALL_ACCESS, FALSE, shm_segment); info_handle = OpenFileMapping(FILE_MAP_ALL_ACCESS, FALSE, shm_info); + } else { + /* IPC_PRIVATE always creates a new segment even if IPC_CREAT flag isn't passed. */ + flags |= IPC_CREAT; } if (!shm_handle && !info_handle) { @@ -662,6 +665,19 @@ TSRM_API int shmget(key_t key, size_t size, int flags) } } + if (key == IPC_PRIVATE) { + /* This should call shm_get with a brand new key id that isn't used yet. See https://man7.org/linux/man-pages/man2/shmget.2.html + * Because shmop_open can be used in userland to attach to shared memory segments, use high positive numbers to avoid accidentally conflicting with userland. */ + key = (php_rand() & 0x3fffffff) + 0x40000000; + for (shm_pair *ptr = TWG(shm); ptr < (TWG(shm) + TWG(shm_size)); ptr++) { + if (ptr->descriptor && ptr->descriptor->shm_perm.key == key) { + key = (php_rand() & 0x3fffffff) + 0x40000000; + ptr = TWG(shm); + continue; + } + } + } + shm = shm_get(key, NULL); if (!shm) { CloseHandle(shm_handle); From 89e24a575c464ca034fc7a012df8d0b6fe5637d9 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Mon, 14 Nov 2022 09:36:50 -0500 Subject: [PATCH 2/3] wip --- TSRM/tsrm_win32.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/TSRM/tsrm_win32.c b/TSRM/tsrm_win32.c index 9a0456c94806a..4457221bd38e1 100644 --- a/TSRM/tsrm_win32.c +++ b/TSRM/tsrm_win32.c @@ -30,6 +30,7 @@ #include "tsrm_win32.h" #include "zend_virtual_cwd.h" #include "win32/ioutil.h" +#include "ext/standard/php_random.h" #ifdef ZTS static ts_rsrc_id win32_globals_id; @@ -668,10 +669,20 @@ TSRM_API int shmget(key_t key, size_t size, int flags) if (key == IPC_PRIVATE) { /* This should call shm_get with a brand new key id that isn't used yet. See https://man7.org/linux/man-pages/man2/shmget.2.html * Because shmop_open can be used in userland to attach to shared memory segments, use high positive numbers to avoid accidentally conflicting with userland. */ - key = (php_rand() & 0x3fffffff) + 0x40000000; + zend_long result; + if (php_random_int(0x4000000, 0x7fff0000, &result, 0) == FAILURE) { + key++; + } else { + key = result; + } + key = result; for (shm_pair *ptr = TWG(shm); ptr < (TWG(shm) + TWG(shm_size)); ptr++) { if (ptr->descriptor && ptr->descriptor->shm_perm.key == key) { - key = (php_rand() & 0x3fffffff) + 0x40000000; + if (php_random_int(0x4000000, 0x7fff0000, &result, 0) == FAILURE) { + key++; + } else { + key = result; + } ptr = TWG(shm); continue; } From 21efeec09f81c6027765b4ca7904114be7f0e441 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Tue, 15 Nov 2022 08:01:45 -0500 Subject: [PATCH 3/3] Address review comments --- TSRM/tsrm_win32.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/TSRM/tsrm_win32.c b/TSRM/tsrm_win32.c index 4457221bd38e1..4b5c58d6982a1 100644 --- a/TSRM/tsrm_win32.c +++ b/TSRM/tsrm_win32.c @@ -30,7 +30,7 @@ #include "tsrm_win32.h" #include "zend_virtual_cwd.h" #include "win32/ioutil.h" -#include "ext/standard/php_random.h" +#include "win32/winutil.h" #ifdef ZTS static ts_rsrc_id win32_globals_id; @@ -614,6 +614,22 @@ TSRM_API int pclose(FILE *stream) #define DESCRIPTOR_PREFIX "TSRM_SHM_DESCRIPTOR:" #define INT_MIN_AS_STRING "-2147483648" + +#define TSRM_BASE_SHM_KEY_ADDRESS 0x20000000 +/* Returns a number between 0x2000_0000 and 0x3fff_ffff. On Windows, key_t is int. */ +static key_t tsrm_choose_random_shm_key(key_t prev_key) { + unsigned char buf[4]; + if (php_win32_get_random_bytes(buf, 4) != SUCCESS) { + return prev_key + 2; + } + uint32_t n = + ((uint32_t)(buf[0]) << 24) | + (((uint32_t)buf[1]) << 16) | + (((uint32_t)buf[2]) << 8) | + (((uint32_t)buf[3])); + return (n & 0x1fffffff) + TSRM_BASE_SHM_KEY_ADDRESS; +} + TSRM_API int shmget(key_t key, size_t size, int flags) {/*{{{*/ shm_pair *shm; @@ -668,21 +684,11 @@ TSRM_API int shmget(key_t key, size_t size, int flags) if (key == IPC_PRIVATE) { /* This should call shm_get with a brand new key id that isn't used yet. See https://man7.org/linux/man-pages/man2/shmget.2.html - * Because shmop_open can be used in userland to attach to shared memory segments, use high positive numbers to avoid accidentally conflicting with userland. */ - zend_long result; - if (php_random_int(0x4000000, 0x7fff0000, &result, 0) == FAILURE) { - key++; - } else { - key = result; - } - key = result; + * Because extensions such as shmop/sysvshm can be used in userland to attach to shared memory segments, use unpredictable high positive numbers to avoid accidentally conflicting with userland. */ + key = tsrm_choose_random_shm_key(TSRM_BASE_SHM_KEY_ADDRESS); for (shm_pair *ptr = TWG(shm); ptr < (TWG(shm) + TWG(shm_size)); ptr++) { if (ptr->descriptor && ptr->descriptor->shm_perm.key == key) { - if (php_random_int(0x4000000, 0x7fff0000, &result, 0) == FAILURE) { - key++; - } else { - key = result; - } + key = tsrm_choose_random_shm_key(key); ptr = TWG(shm); continue; }