* Re: [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
2026-07-10 3:55 [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags Jianlin Shi
@ 2026-07-10 4:06 ` sashiko-bot
2026-07-10 4:43 ` bot+bpf-ci
2026-07-10 22:11 ` Andrii Nakryiko
2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-10 4:06 UTC (permalink / raw)
To: Jianlin Shi; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Returning -EINVAL from a u64-returning BPF helper breaks stable UAPI and results in massive positive values.
--
--- Patch [1]: [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags ---
commit 9dc2317be6516748b67981dfde67e0a14dcc2725
Author: Jianlin Shi <shijianlin11@foxmail.com>
bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
This commit changes the fallback behavior of bpf_ringbuf_query() to return
-EINVAL for unrecognized flags instead of 0. It also updates the UAPI
documentation to reflect this new error code.
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
[ ... ]
> @@ -4694,13 +4694,14 @@ union bpf_attr {
[ ... ]
> - * Requested value, or 0, if *flags* are not recognized.
> + * Requested value on success. **-EINVAL** if *flags* are not recognized.
> *
> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -656,7 +656,7 @@ BPF_CALL_2(bpf_ringbuf_query, struct bpf_map *, map, u64, flags)
> case BPF_RB_OVERWRITE_POS:
> return smp_load_acquire(&rb->overwrite_pos);
> default:
> - return 0;
> + return -EINVAL;
[Severity: Critical]
Does this regression break the stable UAPI for existing programs that might be
relying on the helper returning 0 when probing for unsupported flags?
Since bpf_ringbuf_query() is defined to return a u64, returning -EINVAL (-22)
will be implicitly cast to a very large positive integer (0xffffffffffffffea).
If a BPF program checks whether the returned value is greater than 0,
wouldn't it now wrongly assume a huge size or position instead of
handling it as an error?
Also, because ring buffer positions are monotonic byte counters, could a
perfectly valid position eventually reach 0xffffffffffffffea and become
completely indistinguishable from this new error code?
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_39A7924F2A472D74FA6AD444744E57027905@qq.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
2026-07-10 3:55 [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags Jianlin Shi
2026-07-10 4:06 ` sashiko-bot
@ 2026-07-10 4:43 ` bot+bpf-ci
2026-07-10 22:11 ` Andrii Nakryiko
2 siblings, 0 replies; 6+ messages in thread
From: bot+bpf-ci @ 2026-07-10 4:43 UTC (permalink / raw)
To: shijianlin11, bpf
Cc: andrii, ast, daniel, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 991 bytes --]
> bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
>
> bpf_ringbuf_query() returns 0 for unrecognized flags, which is
> indistinguishable from a successful BPF_RB_AVAIL_DATA query on an empty
> ring buffer. Return -EINVAL instead so invalid flags are reported
> explicitly.
>
> Update the helper documentation accordingly and document
> BPF_RB_OVERWRITE_POS, which was missing from the description list.
>
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
This changes the behavior of an existing helper for unrecognized flags,
which was introduced when bpf_ringbuf_query() was first added. Should
this include:
Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29068374717
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
2026-07-10 3:55 [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags Jianlin Shi
2026-07-10 4:06 ` sashiko-bot
2026-07-10 4:43 ` bot+bpf-ci
@ 2026-07-10 22:11 ` Andrii Nakryiko
2 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2026-07-10 22:11 UTC (permalink / raw)
To: Jianlin Shi; +Cc: bpf, andrii, ast, daniel
On Thu, Jul 9, 2026 at 8:56 PM Jianlin Shi <shijianlin11@foxmail.com> wrote:
>
> bpf_ringbuf_query() returns 0 for unrecognized flags, which is
> indistinguishable from a successful BPF_RB_AVAIL_DATA query on an empty
> ring buffer. Return -EINVAL instead so invalid flags are reported
> explicitly.
>
> Update the helper documentation accordingly and document
> BPF_RB_OVERWRITE_POS, which was missing from the description list.
>
> Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
> ---
> include/uapi/linux/bpf.h | 3 ++-
> kernel/bpf/ringbuf.c | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c8d400b76..b18d5810d 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -4664,13 +4664,14 @@ union bpf_attr {
> * * **BPF_RB_RING_SIZE**: The size of ring buffer.
> * * **BPF_RB_CONS_POS**: Consumer position (can wrap around).
> * * **BPF_RB_PROD_POS**: Producer(s) position (can wrap around).
> + * * **BPF_RB_OVERWRITE_POS**: Overwrite position (can wrap around).
> *
> * Data returned is just a momentary snapshot of actual values
> * and could be inaccurate, so this facility should be used to
> * power heuristics and for reporting, not to make 100% correct
> * calculation.
> * Return
> - * Requested value, or 0, if *flags* are not recognized.
> + * Requested value on success. **-EINVAL** if *flags* are not recognized.
> *
> * long bpf_csum_level(struct sk_buff *skb, u64 level)
> * Description
> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 35ae64ade..e549acdc6 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -656,7 +656,7 @@ BPF_CALL_2(bpf_ringbuf_query, struct bpf_map *, map, u64, flags)
bpf_ringbuf_query() is defined as returning u64, so this -1 is 0xfff...fff.
I think it's fine to keep it at zero. There is no confusion for
BPF_RB_AVAIL_SZ, if you provide correct flags (e.g.,
BPF_RB_AVAIL_DATA), then zero is a valid value, you shouldn't
double-guess anything. For BPF_RB_OVERWRITE_POS (yes, please send a
doc update as a separate patch), on older kernels you will get zero
because kernel doesn't recognize it, but if you are relying on
overwrite_pos, you won't be able to create BPF ringbuf anyways.
tl;dr, yes, it's not ideal, but zero matches better with the semantics
of this specific BPF helper, this was done intentionally back in 2020
and is not an omission.
pw-bot: cr
> case BPF_RB_OVERWRITE_POS:
> return smp_load_acquire(&rb->overwrite_pos);
> default:
> - return 0;
> + return -EINVAL;
> }
> }
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread