netdev - [PATCH] net: multicast: calculate csum of looped-back and forwarded packets

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: <20211019114441.1943131-1-cyril.strejc@skoda.cz>
Date:   Tue, 19 Oct 2021 13:44:41 +0200
From:   Cyril Strejc <cyril.strejc@...da.cz>
To:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, Cyril Strejc <cyril.strejc@...da.cz>
Subject: [PATCH] net: multicast: calculate csum of looped-back and forwarded packets

During a testing of an user-space application which transmits UDP
multicast datagrams and utilizes multicast routing to send the UDP
datagrams out of defined network interfaces, I've found a multicast
router does not fill-in UDP checksum into locally produced, looped-back
and forwarded UDP datagrams, if an original output NIC the datagrams
are sent to has UDP TX checksum offload enabled.

The datagrams are sent malformed out of the NIC the datagrams have been
forwarded to.

It is because:

1. If TX checksum offload is enabled on an output NIC, UDP checksum
   is not calculated by kernel and is not filled into skb data.

2. dev_loopback_xmit(), which is called solely by
   ip_mc_finish_output(), sets skb->ip_summed = CHECKSUM_UNNECESSARY
   unconditionally.

3. Since 35fc92a9 ("[NET]: Allow forwarding of ip_summed except
   CHECKSUM_COMPLETE"), the ip_summed value is preserved during
   forwarding.

4. If ip_summed != CHECKSUM_PARTIAL, checksum is not calculated during
   a packet egress.

We could fix this as follows:

1. Not set CHECKSUM_UNNECESSARY in dev_loopback_xmit(), because it
   is just not true.

2. I assume, the original idea behind setting CHECKSUM_UNNECESSARY in
   dev_loopback_xmit() is to prevent checksum validation of looped-back
   local multicast packets. We can adjust
   __skb_checksum_validate_needed() to handle this as the special case.

Signed-off-by: Cyril Strejc <cyril.strejc@...da.cz>
---
 include/linux/skbuff.h | 4 +++-
 net/core/dev.c         | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 841e2f0f5240..95aa0014c3d6 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4048,7 +4048,9 @@ static inline bool __skb_checksum_validate_needed(struct sk_buff *skb,
 						  bool zero_okay,
 						  __sum16 check)
 {
-	if (skb_csum_unnecessary(skb) || (zero_okay && !check)) {
+	if (skb_csum_unnecessary(skb) ||
+	    (zero_okay && !check) ||
+	    skb->pkt_type == PACKET_LOOPBACK) {
 		skb->csum_valid = 1;
 		__skb_decr_checksum_unnecessary(skb);
 		return false;
diff --git a/net/core/dev.c b/net/core/dev.c
index 7ee9fecd3aff..ba4a0994d97b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3906,7 +3906,6 @@ int dev_loopback_xmit(struct net *net, struct sock *sk, struct sk_buff *skb)
 	skb_reset_mac_header(skb);
 	__skb_pull(skb, skb_network_offset(skb));
 	skb->pkt_type = PACKET_LOOPBACK;
-	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	WARN_ON(!skb_dst(skb));
 	skb_dst_force(skb);
 	netif_rx_ni(skb);
-- 
2.25.1