netdev - Re: [PATCHv3 bpf-next 0/5] Add bpf_sk

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, 27 Mar 2020 14:34:09 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Joe Stringer <joe@...d.net.nz>
Cc:     bpf <bpf@...r.kernel.org>, netdev <netdev@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Martin KaFai Lau <kafai@...com>,
        Roman Mashak <mrv@...atatu.com>
Subject: Re: [PATCHv3 bpf-next 0/5] Add bpf_sk_assign eBPF helper

On 2020-03-27 1:43 p.m., Joe Stringer wrote:
> On Fri, Mar 27, 2020 at 7:14 AM Jamal Hadi Salim <jhs@...atatu.com> wrote:
>>

[..]
>>
>> Trying to understand so if we can port our tc action (and upstream),
>> we would need to replicate:
>>
>>    bpf_sk_assign() - invoked everytime we succeed finding the sk
>>    bpf_sk_release() - invoked everytime we are done processing the sk
> 
> The skb->destructor = sock_pfree() is the balanced other half of
> bpf_sk_assign(), so you shouldn't need to explicitly call
> bpf_sk_release() to handle the refcounting of the assigned socket.
>

per other thread, I think once you factor out what those two functions
call into the kernel proper we will just call those same
things..

> The `bpf_sk_release()` pairs with BPF socket lookup, so if you already
> have other socket lookup code handling the core tproxy logic (looking
> up established, then looking up listen sockets with different tuple)
> then you're presumably already handling that to avoid leaking
> references.
> 

Yes, we have all that code already.

> I think that looking at the test_sk_assign.c BPF program in patch 4/5
> should give you a good sense for what you'd need in the TC action
> logic.

Seems like we are on track. Thanks again for working on this.

cheers,
jamal