-
Notifications
You must be signed in to change notification settings - Fork 224
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
Fix bpf obj get()
behavior.
#3238
Fix bpf obj get()
behavior.
#3238
Conversation
} | ||
|
||
ebpf_assert(reply.header.id == ebpf_operation_id_t::EBPF_OPERATION_GET_PINNED_OBJECT); | ||
|
||
ebpf_handle_t handle = reply.handle; | ||
fd_t fd = _create_file_descriptor_for_handle(handle); | ||
if (fd == ebpf_fd_invalid) { | ||
*fd = _create_file_descriptor_for_handle(handle); |
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 believe _open_osfhandle sets errno. Hence you could update the _create_file_descriptor_for_handle() prototype to return the EBPF error rather than simply assuming it's EBPF_NO_MEMORY.
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.
For coherence, I preserved the same behavior for all _create_file_descriptor_for_handle
calls across the rest of the code, all returning EBPF_NO_MEMORY.
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 looked at https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/open-osfhandle?view=msvc-170, but was not able to find if _open_osfhandle()
sets errno.
If _open_osfhandle()
does set errno, maybe we can file an issue to fix it all the places?
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.
Hmm. libuv/libuv#1420 reported that back in 2017, it didn't.
Fixes #3234
Description
Fixes the behavior of
bpf_obj_get()
to comply with the official specs: https://docs.kernel.org/userspace-api/ebpf/syscall.htmlTesting
CI/CD
Documentation
n.a.
Installation
n.a.