From 19f2133a48fb2dbb30227e63d31edf1179030e3c Mon Sep 17 00:00:00 2001 From: Shankar Seal Date: Fri, 8 Mar 2024 23:27:59 -0800 Subject: [PATCH] fix logic error. --- docs/eBpfExtensions.md | 9 ++++++--- include/ebpf_program_types.h | 14 ++++++++++++-- include/ebpf_windows.h | 2 +- libs/execution_context/ebpf_program.c | 2 +- libs/shared/ebpf_shared_framework.h | 12 +++++++----- libs/shared/shared_common.c | 15 ++++++++------- 6 files changed, 35 insertions(+), 19 deletions(-) diff --git a/docs/eBpfExtensions.md b/docs/eBpfExtensions.md index 073ca13245..47bdbe9370 100644 --- a/docs/eBpfExtensions.md +++ b/docs/eBpfExtensions.md @@ -69,7 +69,7 @@ This is a mandatory header that is common to all data structures needed by eBPF #### `ebpf_program_data_t` Struct The various fields of this structure should be set as follows: -* `header`: Version () and size. +* `header`: Version and size. * `program_info`: Pointer to `ebpf_program_info_t`. * `program_type_specific_helper_function_addresses`: Pointer to `ebpf_helper_function_addresses_t`. This structure provides the helper functions that are exclusive to this program type. @@ -202,8 +202,11 @@ resources allocated in the `ebpf_program_context_create_t` call. ### 2.2 Backward compatibility of the Extension data structures All the extension data structures are versioned. New fields can be added to the end of a data structure to maintain backward compatibility with the existing extensions. In such cases, the size field of the header will be updated but the version of the structure *will not change*. -If the change in data structure is such that it is no longer backward compatible (such as change field type or position), -then the version number will be updated. All previous versions of extension structure will continue to be supported. +Existing eBPF extensions will continue to work without requiring recompilation. + +If the change in data structure is such that it is no longer backward compatible (such as changing field type or position), +then the version number will be updated. Existing eBPF extensions would need to be updated to use the new types. + The set of supported version numbers for the various extension structures are listed in `ebpf_windows.h`. ### 2.3 Program Information NPI Client Attach and Detach Callbacks diff --git a/include/ebpf_program_types.h b/include/ebpf_program_types.h index 744fc602ad..39db0925d4 100644 --- a/include/ebpf_program_types.h +++ b/include/ebpf_program_types.h @@ -9,10 +9,11 @@ #define EBPF_MAX_PROGRAM_DESCRIPTOR_NAME_LENGTH 256 #define EBPF_MAX_HELPER_FUNCTION_NAME_LENGTH 256 +// This is the type definition for the eBPF program type descriptor +// when version is EBPF_PROGRAM_TYPE_DESCRIPTOR_VERSION_0. typedef struct _ebpf_program_type_descriptor { ebpf_extension_header_t header; - // The following fields are available in version EBPF_PROGRAM_TYPE_DESCRIPTOR_VERSION_0 and later. const char* name; const ebpf_context_descriptor_t* context_descriptor; GUID program_type; @@ -20,16 +21,19 @@ typedef struct _ebpf_program_type_descriptor char is_privileged; } ebpf_program_type_descriptor_t; +// This is the type definition for the eBPF helper function prototype +// when version is EBPF_HELPER_FUNCTION_PROTOTYPE_VERSION_0. typedef struct _ebpf_helper_function_prototype { ebpf_extension_header_t header; - // The following fields are available in version EBPF_HELPER_FUNCTION_PROTOTYPE_VERSION_0 and later. uint32_t helper_id; const char* name; ebpf_return_type_t return_type; ebpf_argument_type_t arguments[5]; } ebpf_helper_function_prototype_t; +// This is the type definition for the eBPF program information +// when version is EBPF_PROGRAM_INFO_VERSION_0. typedef struct _ebpf_program_info { ebpf_extension_header_t header; @@ -40,6 +44,8 @@ typedef struct _ebpf_program_info const ebpf_helper_function_prototype_t* global_helper_prototype; } ebpf_program_info_t; +// This is the type definition for the eBPF helper function addresses +// when version is EBPF_HELPER_FUNCTION_ADDRESSES_VERSION_0. typedef struct _ebpf_helper_function_addresses { ebpf_extension_header_t header; @@ -61,6 +67,8 @@ typedef void (*ebpf_program_context_destroy_t)( _Out_writes_bytes_to_opt_(*context_size_out, *context_size_out) uint8_t* context_out, _Inout_ size_t* context_size_out); +// This is the type definition for the eBPF program data +// when version is EBPF_PROGRAM_DATA_VERSION_0. typedef struct _ebpf_program_data { ebpf_extension_header_t header; @@ -75,6 +83,8 @@ typedef struct _ebpf_program_data uint8_t required_irql; ///< IRQL at which the program is invoked. } ebpf_program_data_t; +// This is the type definition for the eBPF program section information +// when version is EBPF_PROGRAM_SECTION_INFORMATION_VERSION_0. typedef struct _ebpf_program_section_info { ebpf_extension_header_t header; diff --git a/include/ebpf_windows.h b/include/ebpf_windows.h index 27162404f5..0e98f17684 100644 --- a/include/ebpf_windows.h +++ b/include/ebpf_windows.h @@ -75,7 +75,7 @@ typedef enum _ebpf_helper_function /** * @brief Header of an eBPF extension data structure. * Every eBPF extension data structure must start with this header. - * New fields can be added to the end eBPF extension data structure + * New fields can be added to the end of an eBPF extension data structure * without breaking backward compatibility. The version field must be * updated only if the new data structure is not backward compatible. */ diff --git a/libs/execution_context/ebpf_program.c b/libs/execution_context/ebpf_program.c index fc2933f6b1..22bedfa90b 100644 --- a/libs/execution_context/ebpf_program.c +++ b/libs/execution_context/ebpf_program.c @@ -246,7 +246,7 @@ _ebpf_program_verify_provider_program_data( goto Done; } - if (program_data->header.size < EBPF_PROGRAM_DATA_MINIMUM_SIZE) { + if (program_data->header.size < EBPF_PROGRAM_DATA_VERSION_0_MINIMUM_SIZE) { EBPF_LOG_MESSAGE_GUID( EBPF_TRACELOG_LEVEL_ERROR, EBPF_TRACELOG_KEYWORD_PROGRAM, diff --git a/libs/shared/ebpf_shared_framework.h b/libs/shared/ebpf_shared_framework.h index d77f38766d..6f625f79dd 100644 --- a/libs/shared/ebpf_shared_framework.h +++ b/libs/shared/ebpf_shared_framework.h @@ -34,15 +34,17 @@ CXPLAT_EXTERN_C_BEGIN #define EBPF_PROGRAM_SECTION_VERSION_LATEST EBPF_PROGRAM_SECTION_VERSION_0 // Minumum length of the eBPF extension program information data structures. -#define EBPF_PROGRAM_DATA_MINIMUM_SIZE (EBPF_OFFSET_OF(ebpf_program_data_t, required_irql) + sizeof(uint8_t)) -#define EBPF_PROGRAM_INFO_MINIMUM_SIZE \ +#define EBPF_PROGRAM_DATA_VERSION_0_MINIMUM_SIZE (EBPF_OFFSET_OF(ebpf_program_data_t, required_irql) + sizeof(uint8_t)) +#define EBPF_PROGRAM_INFO_VERSION_0_MINIMUM_SIZE \ (EBPF_OFFSET_OF(ebpf_program_info_t, global_helper_prototype) + sizeof(ebpf_helper_function_prototype_t*)) -#define EBPF_PROGRAM_TYPE_DESCRIPTOR_MINIMUM_SIZE \ +#define EBPF_PROGRAM_TYPE_DESCRIPTOR_VERSION_0_MINIMUM_SIZE \ (EBPF_OFFSET_OF(ebpf_program_type_descriptor_t, is_privileged) + sizeof(char)) -#define EBPF_CONTEXT_DESCRIPTOR_MINIMUM_SIZE \ +#define EBPF_CONTEXT_DESCRIPTOR_VERSION_0_MINIMUM_SIZE \ (EBPF_OFFSET_OF(ebpf_context_descriptor_t, context_destroy) + sizeof(ebpf_program_context_destroy_t)) -#define EBPF_HELPER_FUNCTION_PROTOTYPE_MINIMUM_SIZE \ +#define EBPF_HELPER_FUNCTION_PROTOTYPE_VERSION_0_MINIMUM_SIZE \ (EBPF_OFFSET_OF(ebpf_helper_function_prototype_t, arguments) + 5 * sizeof(ebpf_argument_type_t)) +#define EBPF_PROGRAM_SECTION_VERSION_0_MINIMUM_SIZE \ + (EBPF_OFFSET_OF(ebpf_program_section_info_t, bpf_attach_type) + sizeof(uint32_t)) // Macro locally suppresses "Unreferenced variable" warning, which in 'Release' builds is treated as an error. #define ebpf_assert_success(x) \ diff --git a/libs/shared/shared_common.c b/libs/shared/shared_common.c index d0c102a634..d5130ca69d 100644 --- a/libs/shared/shared_common.c +++ b/libs/shared/shared_common.c @@ -8,8 +8,9 @@ static bool _ebpf_validate_helper_function_prototype(const ebpf_helper_function_prototype_t* helper_prototype) { return ( - (helper_prototype != NULL) && (helper_prototype->header.size >= EBPF_HELPER_FUNCTION_PROTOTYPE_MINIMUM_SIZE) && - (helper_prototype->header.version <= EBPF_HELPER_FUNCTION_PROTOTYPE_VERSION_LATEST) && + (helper_prototype != NULL) && + (helper_prototype->header.version == EBPF_HELPER_FUNCTION_PROTOTYPE_VERSION_LATEST) && + (helper_prototype->header.size >= EBPF_HELPER_FUNCTION_PROTOTYPE_VERSION_0_MINIMUM_SIZE) && (helper_prototype->name != NULL)); } @@ -38,8 +39,8 @@ _ebpf_validate_program_type_descriptor(_In_ const ebpf_program_type_descriptor_t { return ( (program_type_descriptor != NULL) && - (program_type_descriptor->header.size >= EBPF_PROGRAM_TYPE_DESCRIPTOR_MINIMUM_SIZE) && - (program_type_descriptor->header.version <= EBPF_PROGRAM_TYPE_DESCRIPTOR_VERSION_LATEST) && + (program_type_descriptor->header.version == EBPF_PROGRAM_TYPE_DESCRIPTOR_VERSION_LATEST) && + (program_type_descriptor->header.size >= EBPF_PROGRAM_TYPE_DESCRIPTOR_VERSION_0_MINIMUM_SIZE) && (program_type_descriptor->name != NULL) && _ebpf_validate_context_descriptor(program_type_descriptor->context_descriptor)); } @@ -48,8 +49,8 @@ bool ebpf_validate_program_info(_In_ const ebpf_program_info_t* program_info) { return ( - (program_info != NULL) && (program_info->header.size >= EBPF_PROGRAM_INFO_MINIMUM_SIZE) && - (program_info->header.version <= EBPF_PROGRAM_INFORMATION_VERSION_LATEST) && + (program_info != NULL) && (program_info->header.version == EBPF_PROGRAM_INFORMATION_VERSION_LATEST) && + (program_info->header.size >= EBPF_PROGRAM_INFO_VERSION_0_MINIMUM_SIZE) && _ebpf_validate_program_type_descriptor(program_info->program_type_descriptor) && ebpf_validate_helper_function_prototype_array( program_info->program_type_specific_helper_prototype, @@ -63,7 +64,7 @@ ebpf_validate_program_section_info(_In_ const ebpf_program_section_info_t* secti { return ( (section_info != NULL) && (section_info->header.size >= sizeof(ebpf_program_section_info_t)) && - (section_info->header.version <= EBPF_PROGRAM_SECTION_VERSION_LATEST) && (section_info->section_name != NULL) && + (section_info->header.version == EBPF_PROGRAM_SECTION_VERSION_LATEST) && (section_info->section_name != NULL) && (section_info->program_type != NULL) && (section_info->attach_type != NULL)); }