netdev - Re: [RPC PATCH bpf-next] bpf: implement new BPF_CGROUP_INET_SOCK_POST

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: <f3661770-65dc-05f2-a580-199580eb0b74@fb.com>
Date:   Fri, 15 Jan 2021 09:41:06 -0800
From:   Yonghong Song <yhs@...com>
To:     Stanislav Fomichev <sdf@...gle.com>
CC:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [RPC PATCH bpf-next] bpf: implement new
 BPF_CGROUP_INET_SOCK_POST_CONNECT



On 1/15/21 8:39 AM, Stanislav Fomichev wrote:
> On Thu, Jan 14, 2021 at 8:27 PM Yonghong Song <yhs@...com> wrote:
>>
>>
>>
>> On 1/14/21 7:59 PM, Alexei Starovoitov wrote:
>>> On Thu, Jan 14, 2021 at 7:51 PM Stanislav Fomichev <sdf@...gle.com> wrote:
>>>>>>
>>>>>>           lock_sock(sock->sk);
>>>>>>           err = __inet_stream_connect(sock, uaddr, addr_len, flags, 0);
>>>>>
>>>>> Similarly here, attaching fexit to __inet_stream_connect would execute
>>>>> your BPF program at exactly the same time (and then you can check for
>>>>> err value).
>>>>>
>>>>> Or the point here is to have a more "stable" BPF program type?
>>>> Good suggestion, I can try to play with it, I think it should give me
>>>> all the info I need (I only need sock).
>>>> But yeah, I'd rather prefer a stable interface against stable
>>>> __sk_buff, but maybe fexit will also work.
>>>
>>> Maybe we can add an extension to fentry/fexit that are cgroup scoped?
>>> I think this will solve many such cases.
>>
>> Currently, google is pushing LTO build of the kernel. If this happens,
>> it is possible one global function in one file (say a.c) might be
>> inlined into another file (say b.c). So in this particular case,
>> if the global function is inlined, fentry/fexit approach might be
>> missing some cases? We could mark certain *special purpose* function
>> as non-inline, but not sure whether this is scalable or not.
> For this particular case I don't think it matters, right?
> I'd like to fexit ip4_datagram_connect which is exported symbol,
> it's accessed via proto->connect and there is no way it's
> gonna be inlined. Unless our indirect call macros give clang
> a hint :-/

You are right. It is called through indirect call and by default
compiler won't be able to do inlining. One possibility is profile
guided optimization which often profiles indirect call as well.
They may find the indirect call has one call happening in say
80% and will special case that one and may do inlining. not sure.
I guess kernel build in general is not that advanced. But just
keep in mind that this could happen in distant future.

> 
> I'm in general a bit concerned about using tracing calls for stuff
> like that and depending on the non-uapi, but it's probably
> time to give it a try and see how co-re works :-)

you can filter out based on cgroup id in bpf program. I guess
fexit should work in your use case.