Skip to content

Commit

Permalink
Deregister for helper function change when deleting native module (#2324
Browse files Browse the repository at this point in the history 
)

* Deregister for helper function change when deleting native module

Signed-off-by: Alan Jowett <alanjo@microsoft.com>

* Fix code analysis failure

Signed-off-by: Alan Jowett <alanjo@microsoft.com>

* Fix code analysis failure

Signed-off-by: Alan Jowett <alanjo@microsoft.com>

---------

Signed-off-by: Alan Jowett <alanjo@microsoft.com>
  • Loading branch information
@Alan-Jowett
Alan-Jowett committed Apr 17, 2023
1 parent 664a612 commit 5568378
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 48 deletions.
25 changes: 12 additions & 13 deletions libs/execution_context/ebpf_native.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ typedef struct _ebpf_native_helper_address_changed_context
} ebpf_native_helper_address_changed_context_t;

static ebpf_result_t
_ebpf_native_helper_address_changed(_Inout_ ebpf_program_t* program, _Inout_opt_ void* context);
_ebpf_native_helper_address_changed(
size_t address_count, _In_reads_opt_(address_count) uintptr_t* addresses, _In_opt_ void* context);

static void
_ebpf_native_unload_work_item(_In_opt_ const void* service)
Expand Down Expand Up @@ -182,6 +183,11 @@ _ebpf_native_clean_up_programs(_In_reads_(count_of_programs) ebpf_native_program
{
for (uint32_t i = 0; i < count_of_programs; i++) {
if (programs[i].handle != ebpf_handle_invalid) {
ebpf_program_t* program_object = NULL;
ebpf_assert_success(ebpf_object_reference_by_handle(
programs[i].handle, EBPF_OBJECT_PROGRAM, (ebpf_core_object_t**)&program_object));
ebpf_assert_success(ebpf_program_register_for_helper_changes(program_object, NULL, NULL));
ebpf_object_release_reference((ebpf_core_object_t*)program_object);
ebpf_assert_success(ebpf_handle_close(programs[i].handle));
}
ebpf_free(programs[i].addresses_changed_callback_context);
Expand Down Expand Up @@ -1464,7 +1470,8 @@ ebpf_native_get_count_of_maps(_In_ const GUID* module_id, _Out_ size_t* count_of
}

static ebpf_result_t
_ebpf_native_helper_address_changed(_Inout_ ebpf_program_t* program, _Inout_opt_ void* context)
_ebpf_native_helper_address_changed(
size_t address_count, _In_reads_opt_(address_count) uintptr_t* addresses, _In_opt_ void* context)
{
ebpf_result_t return_value;
ebpf_native_helper_address_changed_context_t* helper_address_changed_context =
Expand All @@ -1479,21 +1486,13 @@ _ebpf_native_helper_address_changed(_Inout_ ebpf_program_t* program, _Inout_opt_
goto Done;
}

helper_function_addresses = ebpf_allocate(helper_count * sizeof(uint64_t));
if (helper_function_addresses == NULL) {
return_value = EBPF_NO_MEMORY;
goto Done;
}

return_value = ebpf_program_get_helper_function_addresses(program, helper_count, helper_function_addresses);

if (return_value != EBPF_SUCCESS) {
if (helper_count != address_count || addresses == NULL) {
return_value = EBPF_INVALID_ARGUMENT;
goto Done;
}

for (size_t i = 0; i < helper_count; i++) {
*(uint64_t*)&(helper_address_changed_context->native_program->entry->helpers[i].address) =
helper_function_addresses[i];
*(uint64_t*)&(helper_address_changed_context->native_program->entry->helpers[i].address) = addresses[i];
}

return_value = EBPF_SUCCESS;
Expand Down
107 changes: 74 additions & 33 deletions libs/execution_context/ebpf_program.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ typedef struct _ebpf_program
{
ebpf_core_object_t object;

ebpf_program_parameters_t parameters;
_Guarded_by_(lock) ebpf_program_parameters_t parameters;

// determinant is parameters.code_type
union
Expand Down Expand Up @@ -82,17 +82,17 @@ typedef struct _ebpf_program
_Guarded_by_(lock) void* helper_function_addresses_changed_context;
} ebpf_program_t;

static ebpf_result_t
_ebpf_program_update_helpers(_Inout_ ebpf_program_t* program);
_Requires_lock_held_(program->lock) static ebpf_result_t _ebpf_program_update_helpers(_Inout_ ebpf_program_t* program);

static ebpf_result_t
_ebpf_program_update_interpret_helpers(_Inout_ ebpf_program_t* program, _Inout_ void* context);
_ebpf_program_update_interpret_helpers(
size_t address_count, _In_reads_(address_count) uintptr_t* addresses, _Inout_ void* context);

static ebpf_result_t
_ebpf_program_update_jit_helpers(_Inout_ ebpf_program_t* program, _Inout_ void* context);
_ebpf_program_update_jit_helpers(
size_t address_count, _In_reads_(address_count) uintptr_t* addresses, _Inout_ void* context);

static ebpf_result_t
_ebpf_program_get_helper_function_address(
_Requires_lock_held_(program->lock) static ebpf_result_t _ebpf_program_get_helper_function_address(
_In_ const ebpf_program_t* program, const uint32_t helper_function_id, uint64_t* address);

_Must_inspect_result_ ebpf_result_t
Expand All @@ -105,8 +105,7 @@ void
ebpf_program_terminate()
{}

static void
_ebpf_program_detach_links(_Inout_ ebpf_program_t* program)
_Requires_lock_not_held_(program->lock) static void _ebpf_program_detach_links(_Inout_ ebpf_program_t* program)
{
EBPF_LOG_ENTRY();
while (!ebpf_list_is_empty(&program->links)) {
Expand Down Expand Up @@ -204,10 +203,13 @@ _ebpf_program_program_info_provider_changed(
goto Exit;
}

ebpf_lock_state_t state = ebpf_lock_lock(&program->lock);

return_value = _ebpf_program_update_helpers(program);
if (return_value != EBPF_SUCCESS) {
EBPF_LOG_MESSAGE(
EBPF_TRACELOG_LEVEL_ERROR, EBPF_TRACELOG_KEYWORD_PROGRAM, "Failed to update helpers for program");
ebpf_lock_unlock(&program->lock, state);
goto Exit;
}

Expand All @@ -216,6 +218,8 @@ _ebpf_program_program_info_provider_changed(
program->global_helper_function_count =
global_helper_function_addresses ? global_helper_function_addresses->helper_function_count : 0;
program->bpf_prog_type = program_data->program_info->program_type_descriptor.bpf_prog_type;

ebpf_lock_unlock(&program->lock, state);
return_value = EBPF_SUCCESS;
}

Expand Down Expand Up @@ -256,6 +260,7 @@ _ebpf_program_free(_In_opt_ _Post_invalid_ ebpf_core_object_t* object)
program->cleanup_work_item = NULL;

ebpf_epoch_schedule_work_item(cleanup_work_item);

EBPF_RETURN_VOID();
}

Expand Down Expand Up @@ -323,8 +328,8 @@ _ebpf_program_epoch_free(_In_ _Post_invalid_ void* context)
EBPF_RETURN_VOID();
}

static ebpf_result_t
_ebpf_program_load_providers(_Inout_ ebpf_program_t* program)
_Requires_lock_not_held_(program->lock) static ebpf_result_t
_ebpf_program_load_providers(_Inout_ ebpf_program_t* program)
{
EBPF_LOG_ENTRY();
ebpf_result_t return_value;
Expand Down Expand Up @@ -647,8 +652,7 @@ ebpf_program_associate_maps(ebpf_program_t* program, ebpf_map_t** maps, uint32_t
EBPF_RETURN_RESULT(result);
}

static ebpf_result_t
_ebpf_program_load_machine_code(
_Requires_lock_held_(program->lock) static ebpf_result_t _ebpf_program_load_machine_code(
_Inout_ ebpf_program_t* program,
_In_opt_ const void* code_context,
_In_reads_(machine_code_size) const uint8_t* machine_code,
Expand Down Expand Up @@ -710,22 +714,48 @@ _ebpf_program_load_machine_code(
EBPF_RETURN_RESULT(return_value);
}

static ebpf_result_t
_ebpf_program_update_helpers(_Inout_ ebpf_program_t* program)
_Requires_lock_held_(program->lock) static ebpf_result_t _ebpf_program_update_helpers(_Inout_ ebpf_program_t* program)
{
ebpf_result_t result = EBPF_SUCCESS;
uintptr_t* helper_function_addresses = NULL;
if (program->parameters.code_type == EBPF_CODE_NATIVE) {
helper_function_addresses =
ebpf_allocate_with_tag(program->helper_function_count * sizeof(uintptr_t), EBPF_POOL_TAG_PROGRAM);
if (helper_function_addresses == NULL) {
result = EBPF_NO_MEMORY;
goto Done;
}

for (uint32_t index = 0; index < program->helper_function_count; index++) {
result = _ebpf_program_get_helper_function_address(
program, program->helper_function_ids[index], &helper_function_addresses[index]);
if (result != EBPF_SUCCESS) {
break;
}
}
}

if (program->helper_function_addresses_changed_callback != NULL) {
return program->helper_function_addresses_changed_callback(
program, program->helper_function_addresses_changed_context);
} else {
return EBPF_SUCCESS;
result = program->helper_function_addresses_changed_callback(
program->helper_function_count,
helper_function_addresses,
program->helper_function_addresses_changed_context);
}
Done:
ebpf_free(helper_function_addresses);
return result;
}

static ebpf_result_t
_ebpf_program_update_interpret_helpers(_Inout_ ebpf_program_t* program, _Inout_ void* context)
_ebpf_program_update_interpret_helpers(
size_t address_count, _In_reads_(address_count) uintptr_t* addresses, _Inout_ void* context)
{
EBPF_LOG_ENTRY();
UNREFERENCED_PARAMETER(context);
UNREFERENCED_PARAMETER(address_count);
UNREFERENCED_PARAMETER(addresses);

ebpf_program_t* program = (ebpf_program_t*)context;
_Analysis_assume_lock_held_(program->lock);
ebpf_result_t result = EBPF_SUCCESS;
size_t index = 0;

Expand Down Expand Up @@ -758,10 +788,13 @@ _ebpf_program_update_interpret_helpers(_Inout_ ebpf_program_t* program, _Inout_
}

static ebpf_result_t
_ebpf_program_update_jit_helpers(_Inout_ ebpf_program_t* program, _Inout_ void* context)
_ebpf_program_update_jit_helpers(
size_t address_count, _In_reads_(address_count) uintptr_t* addresses, _Inout_ void* context)
{
ebpf_result_t return_value;
UNREFERENCED_PARAMETER(context);
UNREFERENCED_PARAMETER(address_count);
UNREFERENCED_PARAMETER(addresses);
ebpf_program_t* program = (ebpf_program_t*)context;
ebpf_program_data_t* program_data = NULL;
const ebpf_helper_function_addresses_t* helper_function_addresses = NULL;
const ebpf_helper_function_addresses_t* global_helper_function_addresses = NULL;
Expand Down Expand Up @@ -915,8 +948,7 @@ _ebpf_program_update_jit_helpers(_Inout_ ebpf_program_t* program, _Inout_ void*
}

#if !defined(CONFIG_BPF_INTERPRETER_DISABLED)
static ebpf_result_t
_ebpf_program_load_byte_code(
_Requires_lock_held_(program->lock) static ebpf_result_t _ebpf_program_load_byte_code(
_Inout_ ebpf_program_t* program, _In_ const ebpf_instruction_t* instructions, size_t instruction_count)
{
EBPF_LOG_ENTRY();
Expand Down Expand Up @@ -954,7 +986,7 @@ _ebpf_program_load_byte_code(
ubpf_set_error_print(program->code_or_vm.vm, ebpf_log_function);

program->helper_function_addresses_changed_callback = _ebpf_program_update_interpret_helpers;
program->helper_function_addresses_changed_context = NULL;
program->helper_function_addresses_changed_context = program;

return_value = _ebpf_program_update_helpers(program);
if (return_value != EBPF_SUCCESS) {
Expand Down Expand Up @@ -997,6 +1029,7 @@ ebpf_program_load_code(
{
EBPF_LOG_ENTRY();
ebpf_result_t result = EBPF_SUCCESS;
ebpf_lock_state_t state = ebpf_lock_lock(&program->lock);
program->parameters.code_type = code_type;
ebpf_assert(
(code_type == EBPF_CODE_NATIVE && code_context != NULL) ||
Expand Down Expand Up @@ -1029,6 +1062,7 @@ ebpf_program_load_code(
}
}

ebpf_lock_unlock(&program->lock, state);
EBPF_RETURN_RESULT(result);
}

Expand Down Expand Up @@ -1137,8 +1171,7 @@ ebpf_program_invoke(
}
}

static ebpf_result_t
_ebpf_program_get_helper_function_address(
_Requires_lock_held_(program->lock) static ebpf_result_t _ebpf_program_get_helper_function_address(
_In_ const ebpf_program_t* program, const uint32_t helper_function_id, uint64_t* address)
{
ebpf_result_t return_value;
Expand Down Expand Up @@ -1248,6 +1281,7 @@ ebpf_program_get_helper_function_addresses(
{
EBPF_LOG_ENTRY();
ebpf_result_t result = EBPF_SUCCESS;
ebpf_lock_state_t state = ebpf_lock_lock((ebpf_lock_t*)&program->lock);

if (program->helper_function_count > addresses_count) {
result = EBPF_INSUFFICIENT_BUFFER;
Expand All @@ -1263,6 +1297,7 @@ ebpf_program_get_helper_function_addresses(
}

Exit:
ebpf_lock_unlock((ebpf_lock_t*)&program->lock, state);
EBPF_RETURN_RESULT(result);
}

Expand Down Expand Up @@ -1572,6 +1607,8 @@ _ebpf_program_initialize_or_verify_program_info_hash(_Inout_ ebpf_program_t* pro
ebpf_cryptographic_hash_t* cryptographic_hash = NULL;
ebpf_helper_id_to_index_t* helper_id_to_index = NULL;
ebpf_program_info_t* program_info = NULL;
ebpf_lock_state_t state = 0;
bool lock_held = false;

result = ebpf_program_get_program_info(program, &program_info);
if (result != EBPF_SUCCESS) {
Expand Down Expand Up @@ -1693,7 +1730,8 @@ _ebpf_program_initialize_or_verify_program_info_hash(_Inout_ ebpf_program_t* pro
if (result != EBPF_SUCCESS) {
goto Exit;
}

state = ebpf_lock_lock(&program->lock);
lock_held = true;
if (program->parameters.program_info_hash_length == 0) {
// This is the first time the program info hash is being computed.
uint8_t* new_hash = ebpf_allocate(output_hash_length);
Expand All @@ -1719,6 +1757,10 @@ _ebpf_program_initialize_or_verify_program_info_hash(_Inout_ ebpf_program_t* pro
result = EBPF_SUCCESS;

Exit:
if (lock_held) {
ebpf_lock_unlock(&program->lock, state);
}

ebpf_free(helper_id_to_index);
ebpf_cryptographic_hash_destroy(cryptographic_hash);
ebpf_program_free_program_info((ebpf_program_info_t*)program_info);
Expand Down Expand Up @@ -1939,15 +1981,14 @@ ebpf_program_execute_test_run(
_Must_inspect_result_ ebpf_result_t
ebpf_program_register_for_helper_changes(
_Inout_ ebpf_program_t* program,
_In_ ebpf_helper_function_addresses_changed_callback_t callback,
_In_opt_ ebpf_helper_function_addresses_changed_callback_t callback,
_In_opt_ void* context)
{
if (program->helper_function_addresses_changed_callback != NULL) {
return EBPF_INVALID_ARGUMENT;
}
ebpf_lock_state_t state = ebpf_lock_lock((ebpf_lock_t*)&program->lock);

program->helper_function_addresses_changed_callback = callback;
program->helper_function_addresses_changed_context = context;
ebpf_lock_unlock((ebpf_lock_t*)&program->lock, state);
return EBPF_SUCCESS;
}

Expand Down
4 changes: 2 additions & 2 deletions libs/execution_context/ebpf_program.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ extern "C"
_In_ ebpf_program_test_run_complete_callback_t callback);

typedef ebpf_result_t (*ebpf_helper_function_addresses_changed_callback_t)(
_Inout_ ebpf_program_t* program, _In_opt_ void* context);
size_t address_count, _In_reads_opt_(address_count) uintptr_t* addresses, _In_opt_ void* context);

/**
* @brief Register to be notified when the helper function addresses change.
Expand All @@ -365,7 +365,7 @@ extern "C"
_Must_inspect_result_ ebpf_result_t
ebpf_program_register_for_helper_changes(
_Inout_ ebpf_program_t* program,
_In_ ebpf_helper_function_addresses_changed_callback_t callback,
_In_opt_ ebpf_helper_function_addresses_changed_callback_t callback,
_In_opt_ void* context);

/**
Expand Down

0 comments on commit 5568378

Please sign in to comment.