Skip to content

Commit

Permalink
Fix #2030: Segmentation fault with autoloaders bailing out
Browse files Browse the repository at this point in the history 
We fix this issue by ensuring that the opline is always connected to its proper execute_data and popping newer entries if they aren't matching.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
  • Loading branch information
@bwoebi
bwoebi committed May 5, 2023
1 parent ea41b28 commit 2ba2357
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 13 deletions.
29 changes: 29 additions & 0 deletions tests/ext/sandbox-regression/class_resolver_bailout_hook.phpt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
--TEST--
Assert bailouts are gracefully handled within class autoloading
--ENV--
DD_TRACE_DEBUG=1
--FILE--
<?php

ddtrace\trace_function("x", function() { class C extends D {} print "Should not appear...\n"; });

function x() {}

spl_autoload_register(function() use (&$s) {
if ($s) {
trigger_error("No D", E_USER_ERROR);
} else {
$s = true;
x();
class B {}
print "Leaving Autoloader\n";
}
});

class A extends B {}

?>
--EXPECTF--
Error raised in ddtrace's closure defined at %s:%d for x(): No D in %s
Leaving Autoloader
Flushing trace of size 2 to send-queue for %s
7 changes: 5 additions & 2 deletions zend_abstract_interface/interceptor/php7/interceptor.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,11 +403,11 @@ static int zai_interceptor_fast_ret_handler(zend_execute_data *execute_data) {
return prev_fast_ret_handler ? prev_fast_ret_handler(execute_data) : ZEND_USER_OPCODE_DISPATCH;
}

void zai_interceptor_check_for_opline_before_exception(void);
void zai_interceptor_check_for_opline_before_exception(zend_execute_data *execute_data);
static user_opcode_handler_t prev_handle_exception_handler;
static int zai_interceptor_handle_exception_handler(zend_execute_data *execute_data) {
// not everything goes through zend_throw_exception_hook, in particular when zend_rethrow_exception alone is used (e.g. during zend_call_function)
zai_interceptor_check_for_opline_before_exception();
zai_interceptor_check_for_opline_before_exception(execute_data);

zai_interceptor_frame_memory *frame_memory;
if (ZEND_HANDLE_EXCEPTION == EX(opline)->opcode && zai_hook_memory_table_find(execute_data, &frame_memory)) {
Expand Down Expand Up @@ -1170,7 +1170,10 @@ static void zai_hook_memory_dtor(zval *zv) {
efree(Z_PTR_P(zv));
}

void zai_interceptor_reset_resolver(void);
void zai_interceptor_activate() {
zai_interceptor_reset_resolver();

zend_hash_init(&zai_hook_memory, 8, nothing, zai_hook_memory_dtor, 0);
}

Expand Down
34 changes: 28 additions & 6 deletions zend_abstract_interface/interceptor/php7/resolver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ PHP_FUNCTION(zai_interceptor_resolve_after_class_alias) {
#define ZAI_INTERCEPTOR_POST_DECLARE_OP 224 // random 8 bit number greater than ZEND_VM_LAST_OPCODE
static zend_op zai_interceptor_post_declare_op;
ZEND_TLS zend_op zai_interceptor_post_declare_ops[4];
struct zai_interceptor_opline { const zend_op *op; struct zai_interceptor_opline *prev; };
struct zai_interceptor_opline { const zend_op *op; const zend_execute_data *execute_data; struct zai_interceptor_opline *prev; };
ZEND_TLS struct zai_interceptor_opline zai_interceptor_opline_before_binding = {0};
static void zai_interceptor_install_post_declare_op(zend_execute_data *execute_data) {
// We replace the current opline *before* it is executed. Thus we need to preserve opline data first:
Expand Down Expand Up @@ -109,10 +109,24 @@ static void zai_interceptor_install_post_declare_op(zend_execute_data *execute_d
zai_interceptor_opline_before_binding.prev = backup;
}
zai_interceptor_opline_before_binding.op = EX(opline);
zai_interceptor_opline_before_binding.execute_data = execute_data;
EX(opline) = zai_interceptor_post_declare_ops;
}

static void zai_interceptor_pop_opline_before_binding() {
static void zai_interceptor_pop_opline_before_binding(zend_execute_data *execute_data) {
// Normally the zai_interceptor_opline_before_binding stack should be in sync with the actual executing stack, but it might not after bailouts
if (execute_data) {
if (zai_interceptor_opline_before_binding.execute_data == execute_data) {
return;
}

while (zai_interceptor_opline_before_binding.prev && zai_interceptor_opline_before_binding.prev->execute_data != execute_data) {
struct zai_interceptor_opline *backup = zai_interceptor_opline_before_binding.prev;
zai_interceptor_opline_before_binding = *backup;
efree(backup);
}
}

struct zai_interceptor_opline *backup = zai_interceptor_opline_before_binding.prev;
if (backup) {
zai_interceptor_opline_before_binding = *backup;
Expand Down Expand Up @@ -171,8 +185,9 @@ static int zai_interceptor_post_declare_handler(zend_execute_data *execute_data)
}
}
// preserve offset
zai_interceptor_pop_opline_before_binding(execute_data);
EX(opline) = zai_interceptor_opline_before_binding.op + (EX(opline) - &zai_interceptor_post_declare_ops[0]);
zai_interceptor_pop_opline_before_binding();
zai_interceptor_pop_opline_before_binding(NULL);
return ZEND_USER_OPCODE_CONTINUE;
} else if (prev_post_declare_handler) {
return prev_post_declare_handler(execute_data);
Expand Down Expand Up @@ -244,10 +259,11 @@ static int zai_interceptor_add_interface_handler(zend_execute_data *execute_data
}
#endif

void zai_interceptor_check_for_opline_before_exception(void) {
void zai_interceptor_check_for_opline_before_exception(zend_execute_data *execute_data) {
if (EG(opline_before_exception) == zai_interceptor_post_declare_ops) {
zai_interceptor_pop_opline_before_binding(execute_data);
EG(opline_before_exception) = zai_interceptor_opline_before_binding.op;
zai_interceptor_pop_opline_before_binding();
zai_interceptor_pop_opline_before_binding(NULL);
}
}

Expand All @@ -256,14 +272,20 @@ static void zai_interceptor_exception_hook(zval *ex) {
zend_function *func = EG(current_execute_data)->func;
if (func && ZEND_USER_CODE(func->type) && EG(current_execute_data)->opline == zai_interceptor_post_declare_ops) {
// called right before setting EG(opline_before_exception), reset to original value to ensure correct throw_op handling
zai_interceptor_pop_opline_before_binding(EG(current_execute_data));
EG(current_execute_data)->opline = zai_interceptor_opline_before_binding.op;
zai_interceptor_pop_opline_before_binding();
zai_interceptor_pop_opline_before_binding(NULL);
}
if (prev_exception_hook) {
prev_exception_hook(ex);
}
}

void zai_interceptor_reset_resolver() {
// reset in case a prior request had a bailout
zai_interceptor_opline_before_binding = (struct zai_interceptor_opline){0};
}

// post startup hook to be after opcache
void zai_interceptor_setup_resolving_post_startup(void) {
prev_compile_file = zend_compile_file;
Expand Down
7 changes: 7 additions & 0 deletions zend_abstract_interface/interceptor/php8/interceptor.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -818,9 +818,16 @@ static void zai_hook_memory_dtor(zval *zv) {
efree(Z_PTR_P(zv));
}

#if PHP_VERSION_ID < 80200
void zai_interceptor_reset_resolver(void);
#endif
void zai_interceptor_activate(void) {
zend_hash_init(&zai_hook_memory, 8, nothing, zai_hook_memory_dtor, 0);
zend_hash_init(&zai_interceptor_implicit_generators, 8, nothing, NULL, 0);

#if PHP_VERSION_ID < 80200
zai_interceptor_reset_resolver();
#endif
}

void zai_interceptor_deactivate(void) {
Expand Down
32 changes: 27 additions & 5 deletions zend_abstract_interface/interceptor/php8/resolver_pre-8_2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ PHP_FUNCTION(zai_interceptor_resolve_after_class_alias) {

static zend_op zai_interceptor_post_declare_op;
ZEND_TLS zend_op zai_interceptor_post_declare_ops[4];
struct zai_interceptor_opline { const zend_op *op; struct zai_interceptor_opline *prev; };
struct zai_interceptor_opline { const zend_op *op; const zend_execute_data *execute_data; struct zai_interceptor_opline *prev; };
ZEND_TLS struct zai_interceptor_opline zai_interceptor_opline_before_binding = {0};
static void zai_interceptor_install_post_declare_op(zend_execute_data *execute_data) {
// We replace the current opline *before* it is executed. Thus we need to preserve opline data first:
Expand Down Expand Up @@ -110,10 +110,24 @@ static void zai_interceptor_install_post_declare_op(zend_execute_data *execute_d
zai_interceptor_opline_before_binding.prev = backup;
}
zai_interceptor_opline_before_binding.op = EX(opline);
zai_interceptor_opline_before_binding.execute_data = execute_data;
EX(opline) = zai_interceptor_post_declare_ops;
}

static void zai_interceptor_pop_opline_before_binding() {
static void zai_interceptor_pop_opline_before_binding(zend_execute_data *execute_data) {
// Normally the zai_interceptor_opline_before_binding stack should be in sync with the actual executing stack, but it might not after bailouts
if (execute_data) {
if (zai_interceptor_opline_before_binding.execute_data == execute_data) {
return;
}

while (zai_interceptor_opline_before_binding.prev && zai_interceptor_opline_before_binding.prev->execute_data != execute_data) {
struct zai_interceptor_opline *backup = zai_interceptor_opline_before_binding.prev;
zai_interceptor_opline_before_binding = *backup;
efree(backup);
}
}

struct zai_interceptor_opline *backup = zai_interceptor_opline_before_binding.prev;
if (backup) {
zai_interceptor_opline_before_binding = *backup;
Expand Down Expand Up @@ -154,8 +168,9 @@ static int zai_interceptor_post_declare_handler(zend_execute_data *execute_data)
}
}
// preserve offset
zai_interceptor_pop_opline_before_binding(execute_data);
EX(opline) = zai_interceptor_opline_before_binding.op + (EX(opline) - &zai_interceptor_post_declare_ops[0]);
zai_interceptor_pop_opline_before_binding();
zai_interceptor_pop_opline_before_binding(NULL);
return ZEND_USER_OPCODE_CONTINUE;
} else if (prev_post_declare_handler) {
return prev_post_declare_handler(execute_data);
Expand Down Expand Up @@ -237,8 +252,9 @@ static user_opcode_handler_t prev_handle_exception_handler;
static int zai_interceptor_handle_exception_handler(zend_execute_data *execute_data) {
// not everything goes through zend_throw_exception_hook, in particular when zend_rethrow_exception alone is used (e.g. during zend_call_function)
if (EG(opline_before_exception) == zai_interceptor_post_declare_ops) {
zai_interceptor_pop_opline_before_binding(execute_data);
EG(opline_before_exception) = zai_interceptor_opline_before_binding.op;
zai_interceptor_pop_opline_before_binding();
zai_interceptor_pop_opline_before_binding(NULL);
}

return prev_handle_exception_handler ? prev_handle_exception_handler(execute_data) : ZEND_USER_OPCODE_DISPATCH;
Expand All @@ -249,14 +265,20 @@ static void zai_interceptor_exception_hook(zend_object *ex) {
zend_function *func = EG(current_execute_data)->func;
if (func && ZEND_USER_CODE(func->type) && EG(current_execute_data)->opline == zai_interceptor_post_declare_ops) {
// called right before setting EG(opline_before_exception), reset to original value to ensure correct throw_op handling
zai_interceptor_pop_opline_before_binding(EG(current_execute_data));
EG(current_execute_data)->opline = zai_interceptor_opline_before_binding.op;
zai_interceptor_pop_opline_before_binding();
zai_interceptor_pop_opline_before_binding(NULL);
}
if (prev_exception_hook) {
prev_exception_hook(ex);
}
}

void zai_interceptor_reset_resolver() {
// reset in case a prior request had a bailout
zai_interceptor_opline_before_binding = (struct zai_interceptor_opline){0};
}

static void *opcache_handle;
static void zai_interceptor_find_opcache_handle(void *ext) {
zend_extension *extension = (zend_extension *)ext;
Expand Down

0 comments on commit 2ba2357

Please sign in to comment.