All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
@ 2026-07-10  3:55 Jianlin Shi
  2026-07-10  4:06 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jianlin Shi @ 2026-07-10  3:55 UTC (permalink / raw)
  To: bpf; +Cc: andrii, ast, daniel

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)
 	case BPF_RB_OVERWRITE_POS:
 		return smp_load_acquire(&rb->overwrite_pos);
 	default:
-		return 0;
+		return -EINVAL;
 	}
 }
 
-- 
2.43.0


^ permalink raw reply related	[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: 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

* Re: [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
       [not found] <20260710035555.3725522-1-shijianlin11@foxmail.com>
@ 2026-07-13  1:31 ` Jianlin Shi
  2026-07-13  1:44 ` Jianlin Shi
  1 sibling, 0 replies; 6+ messages in thread
From: Jianlin Shi @ 2026-07-13  1:31 UTC (permalink / raw)
  To: bpf; +Cc: andrii, ast, daniel



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] bpf: return -EINVAL from bpf_ringbuf_query for unknown flags
       [not found] <20260710035555.3725522-1-shijianlin11@foxmail.com>
  2026-07-13  1:31 ` Jianlin Shi
@ 2026-07-13  1:44 ` Jianlin Shi
  1 sibling, 0 replies; 6+ messages in thread
From: Jianlin Shi @ 2026-07-13  1:44 UTC (permalink / raw)
  To: bpf; +Cc: andrii, ast, daniel

Hi,

Thanks for the review.  I'll drop the -EINVAL change and send a
separate doc-only patch for BPF_RB_OVERWRITE_POS.

Thanks,
Jianlin Shi


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-13  1:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
     [not found] <20260710035555.3725522-1-shijianlin11@foxmail.com>
2026-07-13  1:31 ` Jianlin Shi
2026-07-13  1:44 ` Jianlin Shi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.