netdev - Re: [PATCH bpf 2/2] bpf: selftests: Add non function pointer test to struct

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:   Wed, 10 Feb 2021 18:07:04 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Martin KaFai Lau <kafai@...com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf 2/2] bpf: selftests: Add non function pointer test to struct_ops

On Wed, Feb 10, 2021 at 5:55 PM Martin KaFai Lau <kafai@...com> wrote:
>
> On Wed, Feb 10, 2021 at 02:54:40PM -0800, Andrii Nakryiko wrote:
> > On Wed, Feb 10, 2021 at 1:17 PM Martin KaFai Lau <kafai@...com> wrote:
> > >
> > > On Wed, Feb 10, 2021 at 12:27:38PM -0800, Andrii Nakryiko wrote:
> > > > On Tue, Feb 9, 2021 at 12:11 PM Martin KaFai Lau <kafai@...com> wrote:
> > > > >
> > > > > This patch adds a "void *owner" member.  The existing
> > > > > bpf_tcp_ca test will ensure the bpf_cubic.o and bpf_dctcp.o
> > > > > can be loaded.
> > > > >
> > > > > Signed-off-by: Martin KaFai Lau <kafai@...com>
> > > > > ---
> > > >
> > > > Acked-by: Andrii Nakryiko <andrii@...nel.org>
> > > >
> > > > What will happen if BPF code initializes such non-func ptr member?
> > > > Will libbpf complain or just ignore those values? Ignoring initialized
> > > > members isn't great.
> > > The latter. libbpf will ignore non-func ptr member.  The non-func ptr
> > > member stays zero when it is passed to the kernel.
> > >
> > > libbpf can be changed to copy this non-func ptr value.
> > > The kernel will decide what to do with it.  It will
> > > then be consistent with int/array member like ".name"
> > > and ".flags" where the kernel will verify the value.
> > > I can spin v2 to do that.
> >
> > I was thinking about erroring out on non-zero fields, but if you think
> > it's useful to pass through values, it could be done, but will require
> > more and careful code, probably. So, basically, don't feel obligated
> > to do this in this patch set.
> You meant it needs different handling in copying ptr value
> than copying int/char[]?

Hm.. If we are talking about copying pointer values, then I don't see
how you can provide a valid kernel pointer from the BPF program?...
But if we are talking about copying field values in general, then
you'll need to handle enums, struct/union, etc, no? If int/char[] is
supported (I probably missed that it is), that might be the only
things you'd need to support. So for non function pointers, I'd just
enforce zeroes.