Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deregister for helper function change when deleting native module #2324

Merged
merged 4 commits into from
Apr 18, 2023
Merged

Deregister for helper function change when deleting native module #2324

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6bbf2b9
Deregister for helper function change when deleting native module
Alan-Jowett Apr 13, 2023
6179329
Fix code analysis failure
Alan-Jowett Apr 14, 2023
ab47020
Merge branch 'microsoft:main' into issue2316
Alan-Jowett Apr 14, 2023
ad63e63
Fix code analysis failure
Alan-Jowett Apr 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 12 additions & 13 deletions libs/execution_context/ebpf_native.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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 @@ -180,6 +181,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 @@ -1462,7 +1468,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 @@ -1477,21 +1484,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
106 changes: 73 additions & 33 deletions libs/execution_context/ebpf_program.c
View file
Open in desktop
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,6 +203,8 @@ _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(
Expand All @@ -216,6 +217,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 +259,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 +327,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 +651,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 +713,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 +787,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 +947,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 +985,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 +1028,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 +1061,7 @@ ebpf_program_load_code(
}
}

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

Expand Down Expand Up @@ -1137,8 +1170,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 +1280,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 +1296,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 +1606,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 +1729,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 +1756,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 +1980,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
Open in desktop
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