linux-kernel - Re: [PATCH bpf-next v5 3/6] bpf/helpers: introduce bpf_timer_set_sleepable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 22 Mar 2024 17:50:11 +0100
From: Kumar Kartikeya Dwivedi <memxor@...il.com>
To: Benjamin Tissoires <bentiss@...nel.org>
Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>, 
	bpf@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v5 3/6] bpf/helpers: introduce
 bpf_timer_set_sleepable_cb() kfunc

On Fri, 22 Mar 2024 at 15:57, Benjamin Tissoires <bentiss@...nel.org> wrote:
>
> In this patch, bpf_timer_set_sleepable_cb() is functionally equivalent
> to bpf_timer_set_callback(), to the exception that it enforces
> the timer to be started with BPF_F_TIMER_SLEEPABLE.
>
> But given that bpf_timer_set_callback() is a helper when
> bpf_timer_set_sleepable_cb() is a kfunc, we need to teach the verifier
> about its attached callback.
> Marking that callback as sleepable will be done in a separate patch
>
> Acked-by: Eduard Zingerman <eddyz87@...il.com>
> Signed-off-by: Benjamin Tissoires <bentiss@...nel.org>
>
> ---
> [...]
>
> @@ -19548,6 +19582,28 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>                    desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) {
>                 insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1);
>                 *cnt = 1;
> +       } else if (is_bpf_timer_set_sleepable_cb_impl_kfunc(desc->func_id)) {
> +               /* The verifier will process callback_fn as many times as necessary
> +                * with different maps and the register states prepared by
> +                * set_timer_callback_state will be accurate.
> +                *
> +                * The following use case is valid:
> +                *   map1 is shared by prog1, prog2, prog3.
> +                *   prog1 calls bpf_timer_init for some map1 elements
> +                *   prog2 calls bpf_timer_set_callback for some map1 elements.
> +                *     Those that were not bpf_timer_init-ed will return -EINVAL.
> +                *   prog3 calls bpf_timer_start for some map1 elements.
> +                *     Those that were not both bpf_timer_init-ed and
> +                *     bpf_timer_set_callback-ed will return -EINVAL.
> +                */
> +               struct bpf_insn ld_addrs[2] = {
> +                       BPF_LD_IMM64(BPF_REG_3, (long)env->prog->aux),
> +               };
> +
> +               insn_buf[0] = ld_addrs[0];
> +               insn_buf[1] = ld_addrs[1];
> +               insn_buf[2] = *insn;
> +               *cnt = 3;
>         }

Would be better to handle the fixup of all kfuncs in one place, i.e.
fixup_kfunc_call.

>         return 0;
>  }
>
> --
> 2.44.0
>
>