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.

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

bpf() shim don't follow upstream error conventions #3749

Open
lmb opened this issue Aug 5, 2024 · 1 comment
Open

bpf() shim don't follow upstream error conventions #3749

lmb opened this issue Aug 5, 2024 · 1 comment
Assignees
Labels
bug Something isn't working P1 triaged Discussed in a triage meeting
Milestone

Comments

@lmb
Copy link
Contributor

lmb commented Aug 5, 2024

Describe the bug

The efW libbpf shim uses the convention of returning -1 and setting errno. Upstream libbpf returns errno directly from most functions, except when returning a pointer. See https://libbpf.readthedocs.io/en/latest/api.html

This problem extends to the bpf() syscall wrapper, which doesn't follow Linux ABI for this reason. See https://man7.org/linux/man-pages/man2/bpf.2.html

OS information

No response

Steps taken to reproduce bug

Call a libbpf function and inspect the return code.

Expected behavior

errno should not be modified, and errors returned according to upstream behaviour.

Actual outcome

errno is modified and -1 returned.

Additional details

No response

@lmb lmb added the bug Something isn't working label Aug 5, 2024
@lmb lmb changed the title libbpf shim doesn't follow upstream libbpf error conventions libbpf / npf() shim don't follow upstream error conventions Aug 5, 2024
@lmb lmb changed the title libbpf / npf() shim don't follow upstream error conventions libbpf / bpf() shim don't follow upstream error conventions Aug 5, 2024
@lmb
Copy link
Contributor Author

lmb commented Aug 5, 2024

I think I misunderstood how things work, at least partially:

static inline int
libbpf_err(int ret)
{
if (ret < 0)
errno = -ret;
return ret;
}
static inline int
libbpf_result_err(ebpf_result_t result)
{
return libbpf_err(-ebpf_result_to_errno(result));
}
static inline void*
libbpf_err_ptr(int err)
{
errno = -err;
return NULL;
}

The libbpf wrappers use these functions to return integer errors in the correct way. What confused me is that they also set errno, and some tests inspect errno.

The bpf() wrapper does indeed contain some error returns which aren't according to spec though:

It might make sense to reword this issue to only be about the bpf() wrapper.

@shankarseal shankarseal changed the title libbpf / bpf() shim don't follow upstream error conventions bpf() shim don't follow upstream error conventions Aug 5, 2024
@shankarseal shankarseal added this to the 2408 milestone Aug 5, 2024
@shankarseal shankarseal added triaged Discussed in a triage meeting P1 labels Aug 5, 2024
lmb added a commit to lmb/ebpf-for-windows that referenced this issue Aug 12, 2024
The Linux ABI returns all syscall errors via the function return value,
not via errno.

Fixes microsoft#3749
lmb added a commit to lmb/ebpf-for-windows that referenced this issue Aug 13, 2024
The Linux ABI returns all syscall errors via the function return value,
not via errno.

Fixes microsoft#3749
@shankarseal shankarseal modified the milestones: 2408, 2409 Aug 28, 2024
lmb added a commit to lmb/ebpf-for-windows that referenced this issue Sep 26, 2024
The Linux ABI returns all syscall errors via the function return value,
not via errno.

Fixes microsoft#3749
lmb added a commit to lmb/ebpf-for-windows that referenced this issue Sep 26, 2024
The Linux ABI returns all syscall errors via the function return value,
not via errno.

Fixes microsoft#3749
@shankarseal shankarseal modified the milestones: 2409, 2410 Sep 30, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 4, 2024
* bpf(): do not return errors via errno

The Linux ABI returns all syscall errors via the function return value,
not via errno.

Fixes #3749

* Allow detecting if bpf() command is not implemented

Use SetLastError to indicate to callers that a bpf() command is not
implemented. This avoids polluting the bpf() return value with
platform specific error returns while still allowing detection of
this important case.

* Add forwards and backwards compatibility to bpf() emulation

On Linux, bpf() accepts a bpf_attr which is larger than what the
syscall expects, as long as the unknown fields are all 0. It also
accepts a bpf_attr which is smaller than what it expects, by assuming
that all missing fields are zero.

This allows forwards and backwards compatilibity between old and new
versions of both the Linux kernel and user space tooling.

Implement a similar scheme for the bpf() emulation.

* Return EPERM from bpf() if user is not privileged

On Linux, bpf() returns EPERM if the user doesn't have CAP_BPF. Return
the same error when the user isn't able to open the device handle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1 triaged Discussed in a triage meeting
Projects
None yet
Development

No branches or pull requests

2 participants