netdev - [PATCH net] net: Add check for csum_start in skb_partial_csum

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]

Date:   Mon, 10 Apr 2023 10:21:52 +0800
From:   Lu Wei <luwei32@...wei.com>
To:     <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <asml.silence@...il.com>,
        <imagedong@...cent.com>, <brouer@...hat.com>,
        <keescook@...omium.org>, <jbenc@...hat.com>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: [PATCH net] net: Add check for csum_start in skb_partial_csum_set()

If an AF_PACKET socket is used to send packets through a L3 mode ipvlan
and a vnet header is set via setsockopt() with the option name of
PACKET_VNET_HDR, the value of offset will be nagetive in function
skb_checksum_help() and trigger the following warning:

WARNING: CPU: 3 PID: 2023 at net/core/dev.c:3262
skb_checksum_help+0x2dc/0x390
......
Call Trace:
 <TASK>
 ip_do_fragment+0x63d/0xd00
 ip_fragment.constprop.0+0xd2/0x150
 __ip_finish_output+0x154/0x1e0
 ip_finish_output+0x36/0x1b0
 ip_output+0x134/0x240
 ip_local_out+0xba/0xe0
 ipvlan_process_v4_outbound+0x26d/0x2b0
 ipvlan_xmit_mode_l3+0x44b/0x480
 ipvlan_queue_xmit+0xd6/0x1d0
 ipvlan_start_xmit+0x32/0xa0
 dev_hard_start_xmit+0xdf/0x3f0
 packet_snd+0xa7d/0x1130
 packet_sendmsg+0x7b/0xa0
 sock_sendmsg+0x14f/0x160
 __sys_sendto+0x209/0x2e0
 __x64_sys_sendto+0x7d/0x90

The root cause is:
1. skb->csum_start is set in packet_snd() according vnet_hdr:
   skb->csum_start = skb_headroom(skb) + (u32)start;

   'start' is the offset from skb->data, and mac header has been
   set at this moment.

2. when this skb arrives ipvlan_process_outbound(), the mac header
   is unset and skb_pull is called to expand the skb headroom.

3. In function skb_checksum_help(), the variable offset is calculated
   as:
      offset = skb->csum_start - skb_headroom(skb);

   since skb headroom is expanded in step2, offset is nagetive, and it
   is converted to an unsigned integer when compared with skb_headlen
   and trigger the warning.

In fact the data to be checksummed should not contain the mac header
since the mac header is stripped after a packet leaves L2 layer.
This patch fixes this by adding a check for csum_start to make it
start after the mac header.

Fixes: 52b5d6f5dcf0 ("net: make skb_partial_csum_set() more robust against overflows")
Signed-off-by: Lu Wei <luwei32@...wei.com>
---
 net/core/skbuff.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 1a31815104d6..5e24096076fa 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5232,9 +5232,11 @@ bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
 	u32 csum_end = (u32)start + (u32)off + sizeof(__sum16);
 	u32 csum_start = skb_headroom(skb) + (u32)start;

-	if (unlikely(csum_start > U16_MAX || csum_end > skb_headlen(skb))) {
-		net_warn_ratelimited("bad partial csum: csum=%u/%u headroom=%u headlen=%u\n",
-				     start, off, skb_headroom(skb), skb_headlen(skb));
+	if (unlikely(csum_start > U16_MAX || csum_end > skb_headlen(skb) ||
+		     csum_start < skb->network_header)) {
+		net_warn_ratelimited("bad partial csum: csum=%u/%u headroom=%u headlen=%u network_header=%u\n",
+				     start, off, skb_headroom(skb),
+				     skb_headlen(skb), skb->network_header);
 		return false;
 	}
 	skb->ip_summed = CHECKSUM_PARTIAL;
-- 
2.25.1