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: <CAKH8qBuntApFvGYEs_fU_OAsQeP_Uf2sdrEMAtB4rS6c6fhF9A@mail.gmail.com>
Date:   Mon, 24 Apr 2023 10:06:09 -0700
From:   Stanislav Fomichev <sdf@...gle.com>
To:     Gilad Sever <gilad9366@...il.com>
Cc:     dsahern@...nel.org, 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 Sun, Apr 23, 2023 at 4:41 AM Gilad Sever <gilad9366@...il.com> wrote:
>
>
> On 20/04/2023 19:42, Stanislav Fomichev wrote:
> > On 04/20, Gilad Sever wrote:
> >> When calling socket lookup from L2 (tc, xdp), VRF boundaries aren't
> >> respected. This patchset fixes this by regarding the incoming device's
> >> VRF attachment when performing the socket lookups from tc/xdp.
> >>
> >> The first two patches are coding changes which facilitate this fix by
> >> factoring out the tc helper's logic which was shared with cg/sk_skb
> >> (which operate correctly).
> > Why is not relevant for cgroup/egress? Is it already running with
> > the correct device?
> Yes.
> >
> > Also, do we really need all this refactoring and separate paths?
> > Can we just add that bpf_l2_sdif part to the existing code?
> > It will trigger for tc, but I'm assuming it will be a no-op for cgroup
> > path?
> The reason we preferred the refactoring is to avoid triggering `inet_sdif()`
> from tc/xdp. This is because in our understanding, the IPCB is undefined
> before
> IP processing so it seems incorrect to use `inet_sdif()` from tc/xdp.
>
> We did come up with a different option which could spare most of the
> refactoring
> and still partially separate the two paths:
>
> Pass sdif to __bpf_skc_lookup() but instead of using different functions
> for tc, calculate sdif by calling `dev_sdif()` in bpf_skc_lookup() only when
> netif_is_l3_master() is false. In other words:

[..]

> - 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
>    - cgroup callers never see l3 enslaved devices
>    - inet{,6}_sdif() isn't relevant for non l3 master devices

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.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ