netdev - Re: [PATCH v9 bpf-next 3/5] bpf: Add skb dynptrs

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]

Message-ID: <CAJnrk1YuCUKUmG0s_maz_ZNNCxWhcut-29qmaXghLotqyq2fFA@mail.gmail.com>
Date:   Tue, 31 Jan 2023 13:33:48 -0800
From:   Joanne Koong <joannelkoong@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        Kumar Kartikeya Dwivedi <memxor@...il.com>,
        Kernel Team <kernel-team@...com>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH v9 bpf-next 3/5] bpf: Add skb dynptrs

On Tue, Jan 31, 2023 at 1:11 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Tue, Jan 31, 2023 at 12:48 PM Joanne Koong <joannelkoong@...il.com> wrote:
> > > > >
> > > > > p = bpf_dynptr_slice(dp, off, 16, buf);
> > > > > if (p == NULL) {
> > > > >    /* out of range */
> > > > > } else {
> > > > >    /* work with p directly */
> > > > > }
> > > > >
> > > > > /* if we wrote something to p and it was copied to buffer, write it back */
> > > > > if (p == buf) {
> > > > >     bpf_dynptr_write(dp, buf, 16);
> > > > > }
> > > > >
> > > > >
> > > > > We'll just need to teach verifier to make sure that buf is at least 16
> > > > > byte long.
> > > >
> > > > I'm confused what the benefit of passing in the buffer is. If it's to
> > > > avoid the uncloning, this will still need to happen if the user writes
> > > > back the data to the skb (which will be the majority of cases). If
> > > > it's to avoid uncloning if the user only reads the data of a writable
> > > > prog, then we could add logic in the verifier so that we don't pull
> > > > the data in this case; the uncloning might still happen regardless if
> > > > another part of the program does a direct write. If the benefit is to
> > > > avoid needing to pull the data, then can't the user just use
> > > > bpf_dynptr_read, which takes in a buffer?
> > >
> > > There is no unclone and there is no pull in xdp.
> > > The main idea of this semantics of bpf_dynptr_slice is to make it
> > > work the same way on skb and xdp for _read_ case.
> > > Writes are going to be different between skb and xdp anyway.
> > > In some rare cases the writes can be the same for skb and xdp
> > > with this bpf_dynptr_slice + bpf_dynptr_write logic,
> > > but that's a minor feature addition of the api.
> >
> > bpf_dynptr_read works the same way on skb and xdp. bpf_dynptr_read
> > takes in a buffer as well, so what is the added benefit of
> > bpf_dynptr_slice?
>
> That it doesn't copy most of the time.

Ohh I see, I missed that bpf_dynptr_slice also returns back a ptr.
This makes sense to me now, thanks for clarifying.