netdev - Question re. skb_orphan for TPROXY

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-next>] [day] [month] [year] [list]

Message-ID: <CACAyw9-pYyvkUBOzdD+XQBEKdGGB9foJ5ph5sdjiuE4_uyEoJg@mail.gmail.com>
Date:   Tue, 16 Apr 2019 15:49:17 +0100
From:   Lorenz Bauer <lmb@...udflare.com>
To:     herbert@...dor.apana.org.au
Cc:     netdev@...r.kernel.org
Subject: Question re. skb_orphan for TPROXY

Hello Herbert (and List),

Apologies for contacting you out of the blue. I'm currently trying to
understand how TPROXY works under the hood. As part of this endeavour,
I've stumbled upon the commit attached to this email.

>From the commit message I infer that somewhere, TPROXY relies on a
check of skb->sk == NULL to function. However, I can't figure out
where! I've traced TPROXY from NF_HOOK(NF_INET_PRE_ROUTING) just after
the call to skb_orphan to __inet_lookup_skb / skb_steal_sock called
from the TCP and UDP receive functions, and as far as I can tell there
is no such check. Can you maybe shed some light on this?

The commit is a fix for https://bugzilla.kernel.org/show_bug.cgi?id=13627

commit 71f9dacd2e4d233029e9e956ca3f79531f411827
Author: Herbert Xu <herbert@...dor.apana.org.au>
Date:   Fri Jun 26 19:22:37 2009 -0700

    inet: Call skb_orphan before tproxy activates

    As transparent proxying looks up the socket early and assigns
    it to the skb for later processing, we must drop any existing
    socket ownership prior to that in order to distinguish between
    the case where tproxy is active and where it is not.

    Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
    Signed-off-by: David S. Miller <davem@...emloft.net>

diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 490ce20faf38..db46b4b5b2b9 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -440,6 +440,9 @@ int ip_rcv(struct sk_buff *skb, struct net_device
*dev, struct packet_type *pt,
     /* Remove any debris in the socket control block */
     memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));

+    /* Must drop socket now because of tproxy. */
+    skb_orphan(skb);
+
     return NF_HOOK(PF_INET, NF_INET_PRE_ROUTING, skb, dev, NULL,
                ip_rcv_finish);

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index c3a07d75b5f5..6d6a4277c677 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -139,6 +139,9 @@ int ipv6_rcv(struct sk_buff *skb, struct
net_device *dev, struct packet_type *pt

     rcu_read_unlock();

+    /* Must drop socket now because of tproxy. */
+    skb_orphan(skb);
+
     return NF_HOOK(PF_INET6, NF_INET_PRE_ROUTING, skb, dev, NULL,
                ip6_rcv_finish);
 err:

-- 
Lorenz Bauer  |  Systems Engineer
25 Lavington St., London SE1 0NZ

www.cloudflare.com