[PATCH bpf] bpf: fix argument type in bpf_loop documentation

Matteo Croce posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
include/uapi/linux/bpf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH bpf] bpf: fix argument type in bpf_loop documentation
Posted by Matteo Croce 1 month, 2 weeks ago
From: Matteo Croce <teknoraver@meta.com>

The `index` argument to bpf_loop() is threaded as an u64.
This lead in a subtle verifier denial where clang cloned the argument
in another register[1].

[1] https://github.com/systemd/systemd/pull/34650#issuecomment-2401092895

Signed-off-by: Matteo Croce <teknoraver@meta.com>
---
 include/uapi/linux/bpf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8ab4d8184b9d..874af0186fe8 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5371,7 +5371,7 @@ union bpf_attr {
  *		Currently, the **flags** must be 0. Currently, nr_loops is
  *		limited to 1 << 23 (~8 million) loops.
  *
- *		long (\*callback_fn)(u32 index, void \*ctx);
+ *		long (\*callback_fn)(u64 index, void \*ctx);
  *
  *		where **index** is the current index in the loop. The index
  *		is zero-indexed.
-- 
2.46.0
Re: [PATCH bpf] bpf: fix argument type in bpf_loop documentation
Posted by Alexei Starovoitov 1 month, 2 weeks ago
On Wed, Oct 9, 2024 at 11:05 AM Matteo Croce <technoboy85@gmail.com> wrote:
>
> From: Matteo Croce <teknoraver@meta.com>
>
> The `index` argument to bpf_loop() is threaded as an u64.
> This lead in a subtle verifier denial where clang cloned the argument
> in another register[1].
>
> [1] https://github.com/systemd/systemd/pull/34650#issuecomment-2401092895
>
> Signed-off-by: Matteo Croce <teknoraver@meta.com>
> ---
>  include/uapi/linux/bpf.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 8ab4d8184b9d..874af0186fe8 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5371,7 +5371,7 @@ union bpf_attr {
>   *             Currently, the **flags** must be 0. Currently, nr_loops is
>   *             limited to 1 << 23 (~8 million) loops.
>   *
> - *             long (\*callback_fn)(u32 index, void \*ctx);
> + *             long (\*callback_fn)(u64 index, void \*ctx);

Good catch.
Please update other places too:
static int set_loop_callback_state(struct bpf_verifier_env *env,
                                   struct bpf_func_state *caller,
                                   struct bpf_func_state *callee,
                                   int insn_idx)
{
        /* bpf_loop(u32 nr_loops, void *callback_fn, void *callback_ctx,
         *          u64 flags);
         * callback_fn(u32 index, void *callback_ctx);
         */
        callee->regs[BPF_REG_1].type = SCALAR_VALUE;
..

tools/include/uapi/linux/bpf.h

pw-bot: cr