Skip to content

Commit b5a8798

Browse files
committed
Fix double-free and ZTS-related issues
Building against a ZTS-enabled build of PHP uncovered several issues, including: * Use of missing `tsrm_ls` value. Added calls to TSRMLS_FETCH() where needed. * String keys returned with `value_array_keys` are now duplicated, and as such are correctly passed (and freed) to the caller. * Functions `value_array_index_get` and `value_array_key_get` incorrectly returned references of, rather than copies to, the values required. * Method `receiverSet` incorrectly called `Destroy` on the member value.
1 parent a75a1e5 commit b5a8798

File tree

5 files changed

+51
-7
lines changed

5 files changed

+51
-7
lines changed

context/context.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,12 @@ void context_exec(engine_context *context, char *filename) {
7676

7777
void *context_eval(engine_context *context, char *script) {
7878
int ret;
79+
zval *retval;
7980

8081
#ifdef ZTS
8182
void ***tsrm_ls = context->tsrm_ls;
8283
#endif
8384

84-
zval *retval;
8585
MAKE_STD_ZVAL(retval);
8686

8787
// Attempt to evaluate inline script.
@@ -117,6 +117,10 @@ void context_bind(engine_context *context, char *name, void *value) {
117117
}
118118

119119
void context_destroy(engine_context *context) {
120+
#ifdef ZTS
121+
void ***tsrm_ls = context->tsrm_ls;
122+
#endif
123+
120124
php_request_shutdown((void *) 0);
121125

122126
SG(server_context) = NULL;

receiver/receiver.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,10 @@ static zend_object_value receiver_create(zend_class_entry *class_type TSRMLS_DC)
223223
}
224224

225225
void receiver_define(char *name, void *rcvr) {
226+
#ifdef ZTS
227+
TSRMLS_FETCH();
228+
#endif
229+
226230
zend_class_entry tmp;
227231
INIT_CLASS_ENTRY_EX(tmp, name, strlen(name), NULL);
228232

receiver/receiver.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ func receiverSet(obj unsafe.Pointer, name *C.char, val unsafe.Pointer) {
129129
}
130130

131131
o.values[n].Set(reflect.ValueOf(v.Interface()))
132-
v.Destroy()
133132
}
134133

135134
//export receiverExists

receiver/receiver.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ typedef struct _engine_receiver {
1111
} engine_receiver;
1212

1313
#define receiver_get_pointer(ce, name) \
14-
(void *) Z_LVAL_P(zend_read_static_property(ce, name, sizeof(name) - 1, 1))
14+
(void *) Z_LVAL_P(zend_read_static_property(ce, name, sizeof(name) - 1, 1 \
15+
TSRMLS_CC))
1516

1617
#define receiver_set_pointer(ce, name, ptr) \
1718
zend_declare_property_long(ce, name, sizeof(name) - 1, (long int) ptr, \
18-
ZEND_ACC_STATIC | ZEND_ACC_PRIVATE TSRMLS_CC);
19+
ZEND_ACC_STATIC | ZEND_ACC_PRIVATE TSRMLS_CC)
1920

2021
void receiver_define(char *name, void *rcvr);
2122

value/value.c

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,21 @@ void value_array_key_set(engine_value *arr, const char *key, engine_value *val)
146146
engine_value *value_create_object() {
147147
zval *zv;
148148

149+
#ifdef ZTS
150+
TSRMLS_FETCH();
151+
#endif
152+
149153
MAKE_STD_ZVAL(zv);
150154
object_init(zv);
151155

152156
return value_new(zv);
153157
}
154158

155159
void value_object_property_add(engine_value *obj, const char *key, engine_value *val) {
160+
#ifdef ZTS
161+
TSRMLS_FETCH();
162+
#endif
163+
156164
add_property_zval(obj->value, key, val->value);
157165
}
158166

@@ -208,6 +216,10 @@ char *value_get_string(engine_value *val) {
208216
zval *tmp;
209217
int result;
210218

219+
#ifdef ZTS
220+
TSRMLS_FETCH();
221+
#endif
222+
211223
switch (val->kind) {
212224
case KIND_STRING:
213225
tmp = val->value;
@@ -237,6 +249,10 @@ char *value_get_string(engine_value *val) {
237249
}
238250

239251
unsigned int value_array_size(engine_value *arr) {
252+
#ifdef ZTS
253+
TSRMLS_FETCH();
254+
#endif
255+
240256
switch (arr->kind) {
241257
case KIND_ARRAY:
242258
case KIND_MAP:
@@ -259,6 +275,10 @@ engine_value *value_array_keys(engine_value *arr) {
259275
char *k = NULL;
260276
unsigned long i = 0;
261277

278+
#ifdef ZTS
279+
TSRMLS_FETCH();
280+
#endif
281+
262282
HashTable *h = NULL;
263283
engine_value *keys = value_create_array(value_array_size(arr));
264284

@@ -279,7 +299,7 @@ engine_value *value_array_keys(engine_value *arr) {
279299
add_next_index_long(keys->value, i);
280300
break;
281301
case HASH_KEY_IS_STRING:
282-
add_next_index_string(keys->value, k, 0);
302+
add_next_index_string(keys->value, k, 1);
283303
break;
284304
}
285305

@@ -301,6 +321,10 @@ engine_value *value_array_keys(engine_value *arr) {
301321
void value_array_reset(engine_value *arr) {
302322
HashTable *h = NULL;
303323

324+
#ifdef ZTS
325+
TSRMLS_FETCH();
326+
#endif
327+
304328
switch (arr->kind) {
305329
case KIND_ARRAY:
306330
case KIND_MAP:
@@ -320,6 +344,10 @@ engine_value *value_array_next_get(engine_value *arr) {
320344
zval **tmp = NULL;
321345
HashTable *h = NULL;
322346

347+
#ifdef ZTS
348+
TSRMLS_FETCH();
349+
#endif
350+
323351
switch (arr->kind) {
324352
case KIND_ARRAY:
325353
case KIND_MAP:
@@ -347,6 +375,10 @@ engine_value *value_array_index_get(engine_value *arr, unsigned long idx) {
347375
zval **zv = NULL;
348376
HashTable *h = NULL;
349377

378+
#ifdef ZTS
379+
TSRMLS_FETCH();
380+
#endif
381+
350382
switch (arr->kind) {
351383
case KIND_ARRAY:
352384
case KIND_MAP:
@@ -367,7 +399,7 @@ engine_value *value_array_index_get(engine_value *arr, unsigned long idx) {
367399
}
368400

369401
if (zend_hash_index_find(h, idx, (void **) &zv) == SUCCESS) {
370-
return value_new(*zv);
402+
return value_new_copy(*zv);
371403
}
372404

373405
return value_create_null();
@@ -377,6 +409,10 @@ engine_value *value_array_key_get(engine_value *arr, char *key) {
377409
zval **zv = NULL;
378410
HashTable *h = NULL;
379411

412+
#ifdef ZTS
413+
TSRMLS_FETCH();
414+
#endif
415+
380416
switch (arr->kind) {
381417
case KIND_ARRAY:
382418
case KIND_MAP:
@@ -390,7 +426,7 @@ engine_value *value_array_key_get(engine_value *arr, char *key) {
390426
}
391427

392428
if (zend_hash_find(h, key, strlen(key) + 1, (void **) &zv) == SUCCESS) {
393-
return value_new(*zv);
429+
return value_new_copy(*zv);
394430
}
395431

396432
return value_create_null();

0 commit comments

Comments
 (0)