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

module: ovs: fix return value in raw tracepoint program #332

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

atenart
Copy link
Contributor

@atenart atenart commented Jan 16, 2024

With the upcoming Linux v6.8 and commit c871d0e00f0e ("bpf: enforce precise retval range on program exit") returned value of programs are enforced to be in specific ranges. For tracepoints the range is [0, 0]; meaning programs should always return 0.

This is an issue in Retis as we load hooks as programs and they sometimes return non-0 values. When this happens and the hook is used in a raw tracepoint, the following error message is issued:

At program exit the register R0 has smin=-1 smax=-1 should have been in [0, 0]

So far only a single hook is problematic, kernel_upcall_tp. For completeness, this patch also converts other kernel hooks to not return custom values. This fixes our runtime CI tests on Rawhide.

This alone is not sufficient. While returning custom errors is problematic even for Retis (those are not handled), we allow hooks to return -ENOMSG. This should be handled in another rework. Luckily the only user is quite contained for now and is a kprobe (nft module).

With the upcoming Linux v6.8 and commit c871d0e00f0e ("bpf: enforce
precise retval range on program exit") returned value of programs are
enforced to be in specific ranges. For tracepoints the range is [0, 0];
meaning programs should always return 0.

This is an issue in Retis as we load hooks as programs and they
sometimes return non-0 values. When this happens and the hook is used in
a raw tracepoint, the following error message is issued:

  At program exit the register R0 has smin=-1 smax=-1 should have been in [0, 0]

So far only a single hook is problematic, kernel_upcall_tp. For
completeness, this patch also converts other kernel hooks to not return
custom values. This fixes our runtime CI tests on Rawhide.

This alone is not sufficient. While returning custom errors is
problematic even for Retis (those are not handled), we allow hooks to
return -ENOMSG. This should be handled in another rework. Luckily the
only user is quite contained for now and is a kprobe (nft module).

Signed-off-by: Antoine Tenart <atenart@redhat.com>
@atenart atenart added the run-functional-tests Request functional tests to be run by CI label Jan 16, 2024
@atenart atenart added this to the v1.3.1 milestone Jan 16, 2024
@vlrpl
Copy link
Contributor

vlrpl commented Jan 16, 2024

With the upcoming Linux v6.8 and commit c871d0e00f0e ("bpf: enforce precise retval range on program exit") returned value of programs are enforced to be in specific ranges. For tracepoints the range is [0, 0]; meaning programs should always return 0.

Just a remark based on previous experience, the patch lgtm.
didn't look into it, but this sounds similar to the issue we had in the past with packet filtering.
For that, I considered it a verifier issue as, although it's ok to enforce the retval range, hooks are EXT.
I guess this doesn't happen if the initial empty function returns a value out of the range, also it seems unlikely that the end goal is enforcing the same retval for ebpf2ebpf calls.

This is an issue in Retis as we load hooks as programs and they sometimes return non-0 values. When this happens and the hook is used in a raw tracepoint, the following error message is issued:

At program exit the register R0 has smin=-1 smax=-1 should have been in [0, 0]

So far only a single hook is problematic, kernel_upcall_tp. For completeness, this patch also converts other kernel hooks to not return custom values. This fixes our runtime CI tests on Rawhide.

This alone is not sufficient. While returning custom errors is problematic even for Retis (those are not handled), we allow hooks to return -ENOMSG. This should be handled in another rework. Luckily the only user is quite contained for now and is a kprobe (nft module).

Copy link
Contributor

@vlrpl vlrpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@vlrpl vlrpl merged commit 87379df into main Jan 18, 2024
8 checks passed
@vlrpl vlrpl deleted the at/fix-tp-retval branch January 18, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-functional-tests Request functional tests to be run by CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants