Skip to content

Commit 0b2a887

Browse files
alexdowadalexdowad
committed
Don't use Zend signal handling to protect shared memory in ZendAccelerator.c
Zend signal handling was added in PHP 5.4 to protect against signal handlers running at inopportune times and causing bugs. The details are in this RFC: https://wiki.php.net/rfc/zendsignals In short, the handlers for 7 signals of concern are saved and replaced with a generic handler which delegates to the specific handlers. Inside 'critical sections', however, the generic handler puts all information regarding a signal on a queue and just returns. At the end of the critical section, all pending signals on the queue are processed and the specific handlers are called. However, in PHP 7.1, the `vm_interrupt` flag was added which also protects against script execution timeouts, etc. occurring at wrong times. This eliminated most of the use cases of Zend signal handling. The one which has remained until now is accessing shared memory in OPCache. If that can be eliminated, there will be no need for Zend signal handling any more and a subsystem can be unceremoniously ripped out. The funny thing about the whole idea of Zend signal handling is... it seems to duplicate what Unix kernels already do. Each process/thread in Unix already has a signal mask which can be used to block signals from being delivered at inopportune times. If a signal arrives when it is masked, the kernel will store it and only deliver it once it is unmasked. So rather than going through the whole dance of storing signals on a queue and unqueueing them later... let the kernel do its job.
1 parent e35c163 commit 0b2a887

File tree

1 file changed

+78
-28
lines changed

1 file changed

+78
-28
lines changed

‎ext/opcache/ZendAccelerator.c

Lines changed: 78 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,49 @@ zend_bool file_cache_only = 0; /* process uses file cache only */
118118
zend_bool fallback_process = 0; /* process uses file cache fallback */
119119
#endif
120120

121+
#ifdef HAVE_SIGPROCMASK
122+
static sigset_t mask_all_signals;
123+
124+
# if ZEND_DEBUG
125+
# ifdef ZTS
126+
ZEND_TLS int _signals_masked = 0;
127+
# else
128+
static int _signals_masked = 0;
129+
# endif
130+
# define DEBUG_BLOCK_ALL_SIGNALS() _signals_masked += 1
131+
# define DEBUG_UNBLOCK_ALL_SIGNALS() \
132+
if (--_signals_masked) \
133+
zend_error_noreturn(E_ERROR, "Cannot nest BLOCK_ALL_SIGNALS; it is not re-entrant")
134+
# else
135+
# define DEBUG_BLOCK_ALL_SIGNALS() do {} while (0)
136+
# define DEBUG_UNBLOCK_ALL_SIGNALS() do {} while (0)
137+
# endif
138+
139+
# define BLOCK_ALL_SIGNALS() \
140+
sigset_t _oldmask; \
141+
DEBUG_BLOCK_ALL_SIGNALS(); \
142+
MASK_ALL_SIGNALS()
143+
# define UNBLOCK_ALL_SIGNALS() \
144+
DEBUG_UNBLOCK_ALL_SIGNALS(); \
145+
UNMASK_ALL_SIGNALS()
146+
147+
# ifdef ZTS
148+
# define MASK_ALL_SIGNALS() \
149+
tsrm_sigmask(SIG_BLOCK, &mask_all_signals, &_oldmask)
150+
# define UNMASK_ALL_SIGNALS() \
151+
tsrm_sigmask(SIG_SETMASK, &_oldmask, NULL)
152+
# else
153+
# define MASK_ALL_SIGNALS() \
154+
sigprocmask(SIG_BLOCK, &mask_all_signals, &_oldmask)
155+
# define UNMASK_ALL_SIGNALS() \
156+
sigprocmask(SIG_SETMASK, &_oldmask, NULL)
157+
# endif
158+
159+
#else
160+
# define BLOCK_ALL_SIGNALS() do {} while(0)
161+
# define UNBLOCK_ALL_SIGNALS() do {} while(0)
162+
#endif
163+
121164
static zend_op_array *(*accelerator_orig_compile_file)(zend_file_handle *file_handle, int type);
122165
static int (*accelerator_orig_zend_stream_open_function)(const char *filename, zend_file_handle *handle );
123166
static zend_string *(*accelerator_orig_zend_resolve_path)(const char *filename, size_t filename_len);
@@ -744,7 +787,7 @@ static zend_string* ZEND_FASTCALL accel_replace_string_by_shm_permanent(zend_str
744787

745788
static void accel_use_shm_interned_strings(void)
746789
{
747-
HANDLE_BLOCK_INTERRUPTIONS();
790+
BLOCK_ALL_SIGNALS();
748791
SHM_UNPROTECT();
749792
zend_shared_alloc_lock();
750793

@@ -759,7 +802,7 @@ static void accel_use_shm_interned_strings(void)
759802

760803
zend_shared_alloc_unlock();
761804
SHM_PROTECT();
762-
HANDLE_UNBLOCK_INTERRUPTIONS();
805+
UNBLOCK_ALL_SIGNALS();
763806
}
764807

765808
#ifndef ZEND_WIN32
@@ -1158,7 +1201,7 @@ char *accel_make_persistent_key(const char *path, size_t path_length, int *key_l
11581201

11591202
zend_string *str = accel_find_interned_string(cwd_str);
11601203
if (!str) {
1161-
HANDLE_BLOCK_INTERRUPTIONS();
1204+
BLOCK_ALL_SIGNALS();
11621205
SHM_UNPROTECT();
11631206
zend_shared_alloc_lock();
11641207
str = accel_new_interned_string(zend_string_copy(cwd_str));
@@ -1168,7 +1211,7 @@ char *accel_make_persistent_key(const char *path, size_t path_length, int *key_l
11681211
}
11691212
zend_shared_alloc_unlock();
11701213
SHM_PROTECT();
1171-
HANDLE_UNBLOCK_INTERRUPTIONS();
1214+
UNBLOCK_ALL_SIGNALS();
11721215
}
11731216
if (str) {
11741217
char buf[32];
@@ -1202,7 +1245,7 @@ char *accel_make_persistent_key(const char *path, size_t path_length, int *key_l
12021245

12031246
zend_string *str = accel_find_interned_string(ZCG(include_path));
12041247
if (!str) {
1205-
HANDLE_BLOCK_INTERRUPTIONS();
1248+
BLOCK_ALL_SIGNALS();
12061249
SHM_UNPROTECT();
12071250
zend_shared_alloc_lock();
12081251
str = accel_new_interned_string(zend_string_copy(ZCG(include_path)));
@@ -1211,7 +1254,7 @@ char *accel_make_persistent_key(const char *path, size_t path_length, int *key_l
12111254
}
12121255
zend_shared_alloc_unlock();
12131256
SHM_PROTECT();
1214-
HANDLE_UNBLOCK_INTERRUPTIONS();
1257+
UNBLOCK_ALL_SIGNALS();
12151258
}
12161259
if (str) {
12171260
char buf[32];
@@ -1308,7 +1351,7 @@ int zend_accel_invalidate(const char *filename, size_t filename_len, zend_bool f
13081351
if (force ||
13091352
!ZCG(accel_directives).validate_timestamps ||
13101353
do_validate_timestamps(persistent_script, &file_handle) == FAILURE) {
1311-
HANDLE_BLOCK_INTERRUPTIONS();
1354+
BLOCK_ALL_SIGNALS();
13121355
SHM_UNPROTECT();
13131356
zend_shared_alloc_lock();
13141357
if (!persistent_script->corrupted) {
@@ -1323,7 +1366,7 @@ int zend_accel_invalidate(const char *filename, size_t filename_len, zend_bool f
13231366
}
13241367
zend_shared_alloc_unlock();
13251368
SHM_PROTECT();
1326-
HANDLE_UNBLOCK_INTERRUPTIONS();
1369+
UNBLOCK_ALL_SIGNALS();
13271370
}
13281371
}
13291372

@@ -1838,11 +1881,11 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type)
18381881
}
18391882
}
18401883

1841-
HANDLE_BLOCK_INTERRUPTIONS();
1884+
BLOCK_ALL_SIGNALS();
18421885
SHM_UNPROTECT();
18431886
persistent_script = zend_file_cache_script_load(file_handle);
18441887
SHM_PROTECT();
1845-
HANDLE_UNBLOCK_INTERRUPTIONS();
1888+
UNBLOCK_ALL_SIGNALS();
18461889
if (persistent_script) {
18471890
/* see bug #15471 (old BTS) */
18481891
if (persistent_script->script.filename) {
@@ -2000,13 +2043,13 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
20002043
persistent_script = (zend_persistent_script *)bucket->data;
20012044

20022045
if (key && !persistent_script->corrupted) {
2003-
HANDLE_BLOCK_INTERRUPTIONS();
2046+
BLOCK_ALL_SIGNALS();
20042047
SHM_UNPROTECT();
20052048
zend_shared_alloc_lock();
20062049
zend_accel_add_key(key, key_length, bucket);
20072050
zend_shared_alloc_unlock();
20082051
SHM_PROTECT();
2009-
HANDLE_UNBLOCK_INTERRUPTIONS();
2052+
UNBLOCK_ALL_SIGNALS();
20102053
}
20112054
}
20122055
}
@@ -2051,7 +2094,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
20512094
return NULL;
20522095
}
20532096

2054-
HANDLE_BLOCK_INTERRUPTIONS();
2097+
BLOCK_ALL_SIGNALS();
20552098
SHM_UNPROTECT();
20562099

20572100
/* If script is found then validate_timestamps if option is enabled */
@@ -2114,17 +2157,17 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
21142157
/* No memory left. Behave like without the Accelerator */
21152158
if (ZSMMG(memory_exhausted) || ZCSG(restart_pending)) {
21162159
SHM_PROTECT();
2117-
HANDLE_UNBLOCK_INTERRUPTIONS();
2160+
UNBLOCK_ALL_SIGNALS();
21182161
if (ZCG(accel_directives).file_cache) {
21192162
return file_cache_compile_file(file_handle, type);
21202163
}
21212164
return accelerator_orig_compile_file(file_handle, type);
21222165
}
21232166

21242167
SHM_PROTECT();
2125-
HANDLE_UNBLOCK_INTERRUPTIONS();
2168+
UNBLOCK_ALL_SIGNALS();
21262169
persistent_script = opcache_compile_file(file_handle, type, key, &op_array);
2127-
HANDLE_BLOCK_INTERRUPTIONS();
2170+
BLOCK_ALL_SIGNALS();
21282171
SHM_UNPROTECT();
21292172

21302173
/* Try and cache the script and assume that it is returned from_shared_memory.
@@ -2140,7 +2183,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
21402183
*/
21412184
if (!persistent_script) {
21422185
SHM_PROTECT();
2143-
HANDLE_UNBLOCK_INTERRUPTIONS();
2186+
UNBLOCK_ALL_SIGNALS();
21442187
return op_array;
21452188
}
21462189
if (from_shared_memory) {
@@ -2194,7 +2237,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
21942237
persistent_script->dynamic_members.last_used = ZCG(request_time);
21952238

21962239
SHM_PROTECT();
2197-
HANDLE_UNBLOCK_INTERRUPTIONS();
2240+
UNBLOCK_ALL_SIGNALS();
21982241

21992242
/* Fetch jit auto globals used in the script before execution */
22002243
if (persistent_script->ping_auto_globals_mask) {
@@ -2300,13 +2343,13 @@ static zend_string* persistent_zend_resolve_path(const char *filename, size_t fi
23002343
if (!persistent_script->corrupted) {
23012344
if (key) {
23022345
/* add another "key" for the same bucket */
2303-
HANDLE_BLOCK_INTERRUPTIONS();
2346+
BLOCK_ALL_SIGNALS();
23042347
SHM_UNPROTECT();
23052348
zend_shared_alloc_lock();
23062349
zend_accel_add_key(key, key_length, bucket);
23072350
zend_shared_alloc_unlock();
23082351
SHM_PROTECT();
2309-
HANDLE_UNBLOCK_INTERRUPTIONS();
2352+
UNBLOCK_ALL_SIGNALS();
23102353
} else {
23112354
ZCG(key_len) = 0;
23122355
}
@@ -2403,7 +2446,7 @@ int accel_activate(INIT_FUNC_ARGS)
24032446
}
24042447
#endif
24052448

2406-
HANDLE_BLOCK_INTERRUPTIONS();
2449+
BLOCK_ALL_SIGNALS();
24072450
SHM_UNPROTECT();
24082451

24092452
if (ZCG(counted)) {
@@ -2462,7 +2505,7 @@ int accel_activate(INIT_FUNC_ARGS)
24622505
ZCG(accelerator_enabled) = ZCSG(accelerator_enabled);
24632506

24642507
SHM_PROTECT();
2465-
HANDLE_UNBLOCK_INTERRUPTIONS();
2508+
UNBLOCK_ALL_SIGNALS();
24662509

24672510
if (ZCG(accelerator_enabled) && ZCSG(last_restart_time) != ZCG(last_restart_time)) {
24682511
/* SHM was reinitialized. */
@@ -2889,6 +2932,10 @@ static int accel_startup(zend_extension *extension)
28892932
}
28902933
#endif
28912934

2935+
#ifdef HAVE_SIGPROCMASK
2936+
sigfillset(&mask_all_signals);
2937+
#endif
2938+
28922939
/* no supported SAPI found - disable acceleration and stop initialization */
28932940
if (accel_find_sapi() == FAILURE) {
28942941
accel_startup_ok = 0;
@@ -3157,7 +3204,7 @@ void zend_accel_schedule_restart(zend_accel_restart_reason reason)
31573204
zend_accel_error(ACCEL_LOG_DEBUG, "Restart Scheduled! Reason: %s",
31583205
zend_accel_restart_reason_text[reason]);
31593206

3160-
HANDLE_BLOCK_INTERRUPTIONS();
3207+
BLOCK_ALL_SIGNALS();
31613208
SHM_UNPROTECT();
31623209
ZCSG(restart_pending) = 1;
31633210
ZCSG(restart_reason) = reason;
@@ -3170,7 +3217,7 @@ void zend_accel_schedule_restart(zend_accel_restart_reason reason)
31703217
ZCSG(force_restart_time) = 0;
31713218
}
31723219
SHM_PROTECT();
3173-
HANDLE_UNBLOCK_INTERRUPTIONS();
3220+
UNBLOCK_ALL_SIGNALS();
31743221
}
31753222

31763223
/* this is needed because on WIN32 lock is not decreased unless ZCG(counted) is set */
@@ -4388,6 +4435,9 @@ static int accel_preload(const char *config)
43884435
char *orig_open_basedir;
43894436
size_t orig_map_ptr_last;
43904437
zval *zv;
4438+
#ifdef HAVE_SIGPROCMASK
4439+
sigset_t _oldmask;
4440+
#endif
43914441

43924442
ZCG(enabled) = 0;
43934443
ZCG(accelerator_enabled) = 0;
@@ -4627,7 +4677,7 @@ static int accel_preload(const char *config)
46274677

46284678
zend_shared_alloc_init_xlat_table();
46294679

4630-
HANDLE_BLOCK_INTERRUPTIONS();
4680+
MASK_ALL_SIGNALS();
46314681
SHM_UNPROTECT();
46324682

46334683
/* Store method names first, because they may be shared between preloaded and non-preloaded classes */
@@ -4646,7 +4696,7 @@ static int accel_preload(const char *config)
46464696
ZCSG(preload_script) = preload_script_in_shared_memory(script);
46474697

46484698
SHM_PROTECT();
4649-
HANDLE_UNBLOCK_INTERRUPTIONS();
4699+
UNMASK_ALL_SIGNALS();
46504700

46514701
zend_string_release(filename);
46524702

@@ -4655,7 +4705,7 @@ static int accel_preload(const char *config)
46554705
preload_load();
46564706

46574707
/* Store individual scripts with unlinked classes */
4658-
HANDLE_BLOCK_INTERRUPTIONS();
4708+
MASK_ALL_SIGNALS();
46594709
SHM_UNPROTECT();
46604710

46614711
i = 0;
@@ -4672,7 +4722,7 @@ static int accel_preload(const char *config)
46724722
accel_interned_strings_save_state();
46734723

46744724
SHM_PROTECT();
4675-
HANDLE_UNBLOCK_INTERRUPTIONS();
4725+
UNMASK_ALL_SIGNALS();
46764726

46774727
zend_shared_alloc_destroy_xlat_table();
46784728

0 commit comments

Comments
 (0)