netdev - Re: Removing skb_orphan() from ip_rcv

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: <ab745372-35eb-8bb8-30a4-0e861af27ac2@mojatatu.com>
Date:   Mon, 24 Jun 2019 10:47:15 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Joe Stringer <joe@...d.net.nz>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Florian Westphal <fw@...len.de>
Cc:     netdev <netdev@...r.kernel.org>,
        john fastabend <john.fastabend@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Lorenz Bauer <lmb@...udflare.com>,
        Jakub Sitnicki <jakub@...udflare.com>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: Removing skb_orphan() from ip_rcv_core()

On 2019-06-21 1:58 p.m., Joe Stringer wrote:
> Hi folks, picking this up again..
[..]
> During LSFMM, it seemed like no-one knew quite why the skb_orphan() is
> necessary in that path in the current version of the code, and that we
> may be able to remove it. Florian, I know you weren't in the room for
> that discussion, so raising it again now with a stack trace, Do you
> have some sense what's going on here and whether there's a path
> towards removing it from this path or allowing the skb->sk to be
> retained during ip_rcv() in some conditions?

Sorry - I havent followed the discussion but saw your email over
the weekend and wanted to be at work to refresh my memory on some
code. For maybe 2-3 years we have deployed the tproxy
equivalent as a tc action on ingress (with no netfilter dependency).

And, of course, we had to work around that specific code you are
referring to - we didnt remove it. The tc action code increments
the sk refcount and sets the tc index. The net core doesnt orphan
the skb if a speacial tc index value is set (see attached patch)

I never bothered up streaming the patch because the hack is a bit 
embarrassing (but worked ;->); and never posted the action code
either because i thought this was just us that had this requirement.
I am glad other people see the need for this feature. Is there effort
to make this _not_ depend on iptables/netfilter? I am guessing if you
want to do this from ebpf (tc or xdp) that is a requirement.
Our need was with tcp at the time; so left udp dependency on netfilter
alone.

cheers,
jamal

View attachment "tp_tcindex.patch" of type "text/x-patch" (1928 bytes)