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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAADnVQ+N76ed0h9GJyQfVQiN2pmcqJdjeM5rOPdFv2LfZ9eahQ@mail.gmail.com>
Date:   Tue, 31 Jan 2023 13:10:49 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Joanne Koong <joannelkoong@...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 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.