Skip to content

Commit 281ad6d

Browse files
committed
Fixes issue colder#3
Separate zvals appropriately to avoid access-after-free, re-implement acquire-release using add/del_ref handlers.
1 parent 4762410 commit 281ad6d

File tree

1 file changed

+21
-9
lines changed

1 file changed

+21
-9
lines changed

wr_weakref.c

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,16 @@
3333
static void wr_weakref_ref_dtor(void *ref_object, zend_object_handle ref_handle, zend_object *wref_obj TSRMLS_DC) { /* {{{ */
3434
wr_weakref_object *wref = (wr_weakref_object *)wref_obj;
3535
wref->valid = 0;
36-
wref->ref = NULL;
3736
}
3837
/* }}} */
3938

4039
static int wr_weakref_ref_acquire(wr_weakref_object *intern TSRMLS_DC) /* {{{ */
4140
{
4241
if (intern->valid) {
43-
Z_ADDREF_P(intern->ref);
42+
if (intern->acquired == 0) {
43+
// We need to register that ref so that the object doesn't get collected
44+
Z_OBJ_HANDLER_P(intern->ref, add_ref)(intern->ref TSRMLS_CC);
45+
}
4446
intern->acquired++;
4547
return SUCCESS;
4648
} else {
@@ -52,17 +54,19 @@ static int wr_weakref_ref_acquire(wr_weakref_object *intern TSRMLS_DC) /* {{{ */
5254
static int wr_weakref_ref_release(wr_weakref_object *intern TSRMLS_DC) /* {{{ */
5355
{
5456
if (intern->valid && (intern->acquired > 0)) {
55-
zval *ref_tmp = intern->ref;
56-
zval_ptr_dtor(&ref_tmp);
5757
intern->acquired--;
58+
if (intern->acquired == 0) {
59+
// We need to register that ref so that the object doesn't get collected
60+
Z_OBJ_HANDLER_P(intern->ref, del_ref)(intern->ref TSRMLS_CC);
61+
}
5862
return SUCCESS;
5963
} else {
6064
return FAILURE;
6165
}
6266
}
6367
/* }}} */
6468

65-
static void wr_weakref_object_free_storage(void *object TSRMLS_DC) /* {{{ */
69+
static void wr_weakref_object_free_storage(void *object TSRMLS_DC ZEND_FILE_LINE_DC) /* {{{ */
6670
{
6771
wr_weakref_object *intern = (wr_weakref_object *)object;
6872

@@ -78,6 +82,11 @@ static void wr_weakref_object_free_storage(void *object TSRMLS_DC) /* {{{ */
7882
wr_store_detach((zend_object *)intern, Z_OBJ_HANDLE_P(intern->ref) TSRMLS_CC);
7983
}
8084

85+
if (intern->ref) {
86+
Z_TYPE_P(intern->ref) = IS_NULL;
87+
zval_ptr_dtor(&intern->ref);
88+
}
89+
8190
zend_object_std_dtor(&intern->std TSRMLS_CC);
8291

8392
efree(object);
@@ -111,7 +120,8 @@ static zend_object_value wr_weakref_object_new_ex(zend_class_entry *class_type,
111120
int acquired = 0;
112121

113122
intern->valid = other->valid;
114-
intern->ref = other->ref;
123+
ALLOC_INIT_ZVAL(intern->ref);
124+
ZVAL_COPY_VALUE(intern->ref, other->ref);
115125
wr_store_attach((zend_object *)intern, wr_weakref_ref_dtor, other->ref TSRMLS_CC);
116126

117127
for (acquired = 0; acquired < other->acquired; acquired++) {
@@ -134,7 +144,7 @@ static zend_object_value wr_weakref_object_new_ex(zend_class_entry *class_type,
134144
intern->acquired = 0;
135145
}
136146

137-
retval.handle = zend_objects_store_put(intern, (zend_objects_store_dtor_t)zend_objects_destroy_object, wr_weakref_object_free_storage, NULL TSRMLS_CC);
147+
retval.handle = zend_objects_store_put(intern, (zend_objects_store_dtor_t)zend_objects_destroy_object, (zend_objects_free_object_storage_t)wr_weakref_object_free_storage, NULL TSRMLS_CC);
138148
retval.handlers = &wr_handler_WeakRef;
139149

140150
return retval;
@@ -259,9 +269,11 @@ PHP_METHOD(WeakRef, __construct)
259269

260270
intern = (wr_weakref_object *)zend_object_store_get_object(object TSRMLS_CC);
261271

262-
intern->ref = ref;
272+
ALLOC_INIT_ZVAL(intern->ref);
273+
274+
ZVAL_COPY_VALUE(intern->ref, ref);
263275

264-
wr_store_attach((zend_object *)intern, wr_weakref_ref_dtor, ref TSRMLS_CC);
276+
wr_store_attach((zend_object *)intern, wr_weakref_ref_dtor, intern->ref TSRMLS_CC);
265277

266278
intern->valid = 1;
267279
}

0 commit comments

Comments
 (0)