Isolate eBPF store APIs into atomic km & um libraries. #2690
Conversation
| *provider_key = NULL; | ||
|
|
||
| // Open (or create) root eBPF registry path. | ||
| #ifdef USER_MODE |
There was a problem hiding this comment.
suggestion define ebpf_store_root_key differently for user mode and kernel mode, then you dont have to differentiate in the ebpf_create_registry_key call.
There was a problem hiding this comment.
The "sub-key" would also need to be part of the differentiation as well.
I refactored all the existing so that the ebpf_store_root_key and the new ebpf_store_root_sub_key variables are now atomically defined in their respective KM/UM ebpf_registry_helper libraries (and not in the projects that uses them, which was not ideal), this also further simplifies the code that uses the libraries.
There was a problem hiding this comment.
I'd asked why they're different in the first place, and I still haven't heard any answer to that. Whatever the answer is, it should be documented in // comments at least, in the file(s).
There was a problem hiding this comment.
By looking to the code, it boils down to the different APIs used or KM/UM, i.e. ZwCreateKey and RegCreateKeyExA respectively. The first, takes a NULL (for OBJECT_ATTRIBUTES->RootDirectory) to use the given path as a fully qualified path, whereas the second builds the fully qualified path from predefined keys (https://learn.microsoft.com/en-us/windows/win32/sysinfo/predefined-keys):
- https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-zwcreatekey
- https://learn.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regcreatekeyexa
But I'd still call out @saxena-anurag for the actual history of this implementation.
There was a problem hiding this comment.
I think what Gianni mentioned was the reason to use #ifdef USER_MODE earlier -- to wrap 2 different registry APIs in a single common API (ebpf_create_registry_key())
There was a problem hiding this comment.
ZwCreateKey exists in user-mode too, so is not a reason to have a difference.
| #include "ebpf_store_helper.h" | ||
| #include "ebpf_windows.h" | ||
|
|
||
| #define IS_SUCCESS(x) (x == EBPF_SUCCESS) |
There was a problem hiding this comment.
non blocking comment: This macro was earlier defined as the UM and KM APIs dealt with different return types, but that is not the case now. So maybe this can be removed.
There was a problem hiding this comment.
sure, that can eventually be replaced by its definition, if needed.
| ebpf_close_registry_key(ebpf_store_key_t key); | ||
|
|
||
| _Must_inspect_result_ ebpf_result_t | ||
| ebpf_write_registry_value_binary( |
There was a problem hiding this comment.
nit: is it possible to merge this and the KM registry_helper.h into a single header file (but still have 2 different implementations / .c files)?
There was a problem hiding this comment.
UM & KM have different functions (i.e. UM implements more helpers), so that'd require using preprocessor vars...
Closes #2136
Description
This PR:
ebpf_store_helper_kmandebpf_store_helper_umrespectively, while maintaining a common global header,\include\ebpf_store_helper.h.Testing
CI/CD
Documentation
eBpfExtensions.md