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: <d2692f14-6ac7-1335-3359-d397fbe1676f@gmail.com>
Date:   Mon, 24 Jun 2019 18:49:14 +0200
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>,
        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 6/24/19 7:47 AM, Jamal Hadi Salim wrote:
> 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.
> 


Well, I would simply remove the skb_orphan() call completely.