linux-kernel - Re: [RFC PATCH bpf-next 1/2] bpf: Add open-coded style iterator kfuncs for bpf dynamic pointers

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] [day] [month] [year] [list]

Message-ID: <CAEf4BzYW9NM5WeFMpJwS0krsHgV9TfkXNon3PVfBC_+XYKWqVw@mail.gmail.com>
Date: Fri, 6 Sep 2024 14:33:02 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Juntong Deng <juntong.deng@...look.com>
Cc: ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com, 
	andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, memxor@...il.com, snorcht@...il.com, 
	bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH bpf-next 1/2] bpf: Add open-coded style iterator
 kfuncs for bpf dynamic pointers

On Tue, Sep 3, 2024 at 3:08 PM Juntong Deng <juntong.deng@...look.com> wrote:
>
> This patch adds open coded style bpf dynamic pointer iterator kfuncs
> bpf_iter_dynptr_{new,next,destroy} for iterating over all data in the
> memory region referenced by dynamic pointer.
>
> The idea of bpf dynamic pointer iterator comes from skb data iterator
> [0], we need a way to get all the data in skb. Adding iterator for bpf
> dynamic pointer is a more general choice than adding separate skb data
> iterator.
>
> Each iteration (next) copies the data to the specified buffer and
> updates the offset, with the pointer to the length of the read data
> (errno if errors occurred) as the return value. Note that the offset
> in iterator does not affect the offset in dynamic pointer.
>
> The bpf dynamic pointer iterator has a getter kfunc,
> bpf_iter_dynptr_get_last_offset, which is used to get the offset
> of the last iteration.
>
> The bpf dynamic pointer iterator has a setter kfunc,
> bpf_iter_dynptr_set_buffer, which is used to set the buffer and buffer
> size to be used when copying data in the iteration.
>
> [0]: https://lore.kernel.org/bpf/AM6PR03MB5848DE102F47F592D8B479E999A52@AM6PR03MB5848.eurprd03.prod.outlook.com/
>
> Signed-off-by: Juntong Deng <juntong.deng@...look.com>
> ---
>  kernel/bpf/helpers.c | 110 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 110 insertions(+)
>

Why can't you implement all this using just normal numbers iterator
(bpf_for() loop) and using bpf_dynptr_size(), bpf_dynptr_slice(), etc,
generic helpers.

Seeing that you have to add bpf_iter_dynptr_set_buffer() and
bpf_iter_dynptr_get_last_offset() suggests that this might not be the
best fit for an open-coded iterator. But again, there doesn't seem to
be a real need. dynptr API is generic and usable enough to do whatever
you need without adding more code to the kernel.


[...]