-
Notifications
You must be signed in to change notification settings - Fork 241
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.
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
Add process creation and destruction hooks to netebpfext #3307
Conversation
f43bef5
to
f61aaa1
Compare
Signed-off-by: Alan Jowett <[email protected]>
Signed-off-by: Alan Jowett <[email protected]>
Signed-off-by: Alan Jowett <[email protected]>
* | ||
* @param[in] context \ref process_md_t | ||
* @return STATUS_SUCCESS to permit the operation, or a failure NTSTATUS value to deny the operation. | ||
* Value of STATUS_SUCCESS is 0x0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems odd to say what the value of STATUS_SUCCESS is without saying what range of values are "a failure". I'd either remove line 295 (my preference), or else say more... e.g., failure values have the high bit set.
Still if the point is to document that values 0x1 through 0x7fffffff are not legal to return, the hook still has to do something if they are returned. E.g., what happens if the BPF program returns STATUS_PENDING?
net_ebpf_extension_hook_client_t* client_context = | ||
net_ebpf_extension_hook_get_next_attached_client(_ebpf_process_hook_provider_context, NULL); | ||
while (client_context != NULL) { | ||
NTSTATUS status = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NTSTATUS status = 0; | |
NTSTATUS status = STATUS_SUCCESS; |
NET_EBPF_EXT_TRACELOG_KEYWORD_PROCESS, | ||
"net_ebpf_extension_hook_client_enter_rundown failed"); | ||
} | ||
// If the client returns a non-zero value, stop calling the other clients. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NT_SUCCESS(status) and "non-zero value" are not the same thing. See https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/87fba13e-bf06-450e-83b1-9241dc81e781
uint64_t some_value; | ||
} fake_eprocess = {}; | ||
|
||
usersime_invoke_process_creation_notify_routine( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't "usersime" be "usersim"?
test_process_client_context_t* client_context = (test_process_client_context_t*)client_process_context; | ||
|
||
client_context->process_context = *process_context; | ||
*result = STATUS_ACCESS_DENIED; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also recommend adding a unit test for a program that returns a value that is neither STATUS_SUCCESS nor a failure code. E.g., STATUS_PENDING.
Description
Add support for invoking new eBPF program type on process creation and termination.
Testing
CI/CD
Documentation
TBD
Installation
No.