Skip to content

Commit 7249e7e

Browse files
Fix OPcache memory protection race under ZTS (#23081)
1 parent 2a2b337 commit 7249e7e

2 files changed

Lines changed: 41 additions & 8 deletions

File tree

ext/opcache/ZendAccelerator.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ typedef struct _zend_accel_globals {
199199
bool counted; /* the process uses shared memory */
200200
bool enabled;
201201
bool locked; /* thread obtained exclusive lock */
202+
#ifdef ZTS
203+
uint32_t unprotect_depth;
204+
#endif
202205
bool accelerator_enabled; /* accelerator enabled for current request */
203206
bool pcre_reseted;
204207
zend_accel_directives accel_directives;

ext/opcache/zend_shared_alloc.c

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ static const char *g_shared_model;
5555
/* pointer to globals allocated in SHM and shared across processes */
5656
ZEND_EXT_API zend_smm_shared_globals *smm_shared_globals;
5757

58+
#ifdef ZTS
59+
static MUTEX_T zts_protect_lock;
60+
static uint32_t zts_unprotected_threads;
61+
#endif
62+
5863
#ifndef ZEND_WIN32
5964
#ifdef ZTS
6065
static MUTEX_T zts_lock;
@@ -184,6 +189,11 @@ int zend_shared_alloc_startup(size_t requested_size, size_t reserved_size)
184189
int res = ALLOC_FAILURE;
185190
int i;
186191

192+
#ifdef ZTS
193+
zts_protect_lock = tsrm_mutex_alloc();
194+
zts_unprotected_threads = 0;
195+
#endif
196+
187197
/* shared_free must be valid before we call zend_shared_alloc()
188198
* - make it temporarily point to a local variable
189199
*/
@@ -338,6 +348,9 @@ void zend_shared_alloc_shutdown(void)
338348
tsrm_mutex_free(zts_lock);
339349
# endif
340350
#endif
351+
#ifdef ZTS
352+
tsrm_mutex_free(zts_protect_lock);
353+
#endif
341354
}
342355

343356
static size_t zend_shared_alloc_get_largest_free_block(void)
@@ -625,25 +638,37 @@ const char *zend_accel_get_shared_model(void)
625638

626639
void zend_accel_shared_protect(bool protected)
627640
{
628-
#ifdef HAVE_MPROTECT
641+
#if defined(HAVE_MPROTECT) || defined(ZEND_WIN32)
629642
int i;
630643

631644
if (!smm_shared_globals) {
632645
return;
633646
}
634647

648+
# ifdef ZTS
649+
/* Memory protection is process-wide, so overlapping writers must be tracked across threads. */
650+
tsrm_mutex_lock(zts_protect_lock);
651+
if (protected) {
652+
if (ZCG(unprotect_depth) && --ZCG(unprotect_depth) == 0) {
653+
ZEND_ASSERT(zts_unprotected_threads > 0);
654+
zts_unprotected_threads--;
655+
}
656+
if (zts_unprotected_threads) {
657+
tsrm_mutex_unlock(zts_protect_lock);
658+
return;
659+
}
660+
} else if (ZCG(unprotect_depth)++ == 0) {
661+
zts_unprotected_threads++;
662+
}
663+
# endif
664+
665+
# ifdef HAVE_MPROTECT
635666
const int mode = protected ? PROT_READ : PROT_READ|PROT_WRITE;
636667

637668
for (i = 0; i < ZSMMG(shared_segments_count); i++) {
638669
mprotect(ZSMMG(shared_segments)[i]->p, ZSMMG(shared_segments)[i]->end, mode);
639670
}
640-
#elif defined(ZEND_WIN32)
641-
int i;
642-
643-
if (!smm_shared_globals) {
644-
return;
645-
}
646-
671+
# elif defined(ZEND_WIN32)
647672
const int mode = protected ? PAGE_READONLY : PAGE_READWRITE;
648673

649674
for (i = 0; i < ZSMMG(shared_segments_count); i++) {
@@ -652,6 +677,11 @@ void zend_accel_shared_protect(bool protected)
652677
zend_accel_error_noreturn(ACCEL_LOG_ERROR, "Failed to protect memory");
653678
}
654679
}
680+
# endif
681+
682+
# ifdef ZTS
683+
tsrm_mutex_unlock(zts_protect_lock);
684+
# endif
655685
#endif
656686
}
657687

0 commit comments

Comments
 (0)