netdev - Re: [PATCH bpf,v2 0/4] Socket lookup BPF API from tc/xdp ingress does not respect VRF bindings.

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]

Date:   Mon, 24 Apr 2023 19:39:01 -0600
From:   David Ahern <dsahern@...nel.org>
To:     Stanislav Fomichev <sdf@...gle.com>,
        Gilad Sever <gilad9366@...il.com>
Cc:     martin.lau@...ux.dev, daniel@...earbox.net,
        john.fastabend@...il.com, ast@...nel.org, andrii@...nel.org,
        song@...nel.org, yhs@...com, kpsingh@...nel.org, haoluo@...gle.com,
        jolsa@...nel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, mykolal@...com,
        shuah@...nel.org, hawk@...nel.org, joe@...d.net.nz,
        eyal.birger@...il.com, shmulik.ladkani@...il.com,
        bpf@...r.kernel.org, netdev@...r.kernel.org,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf,v2 0/4] Socket lookup BPF API from tc/xdp ingress does
 not respect VRF bindings.

On 4/24/23 11:06 AM, Stanislav Fomichev wrote:
>> - xdp callers would check the device's l3 enslaved state using the new
>> `dev_sdif()`
>> - sock_addr callers would use inet{,6}_sdif() as they did before
>> - cg/tc share the same code path, so when netif_is_l3_master() is true
>>    use inet{,6}_sdif() and when it is false use dev_sdif(). this relies
>> on the following
>>    assumptions:
>>    - tc programs don't run on l3 master devices

this can happen, but I am not sure how prevalent a use case.

>>    - cgroup callers never see l3 enslaved devices

egress definitely, not sure on ingress. The code resets the skb->dev
back to the original device in a lot of places in the ip/ipv6 code now.
And ipv6 brings up LLAs and those did not get the device switch so it
could be fairly common.

>>    - inet{,6}_sdif() isn't relevant for non l3 master devices

sdif should be 0 and not matched if a netdev is not a l3mdev port.

BTW, in skimming the patches, I noticed patch 3 has bpf_l2_sdif which
seems an odd name to me. It returns a layer 3 device index, not a layer
2 which would be a bridge port. I would stick to the l3 naming for
consistency.

> 
> Yeah, that's what I was assuming we should be able to do..
> But we probably need somebody who understands this part better than me
> to say whether the above are safe..
> 
> If nobody comments, ignore me and do a v2 with your original approach.