netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lu Wei <luwei32@huawei.com>
To: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
	<pabeni@redhat.com>, <asml.silence@gmail.com>,
	<imagedong@tencent.com>, <brouer@redhat.com>,
	<keescook@chromium.org>, <jbenc@redhat.com>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: [PATCH net] net: Add check for csum_start in skb_partial_csum_set()
Date: Mon, 10 Apr 2023 10:21:52 +0800	[thread overview]
Message-ID: <20230410022152.4049060-1-luwei32@huawei.com> (raw)

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@huawei.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


             reply	other threads:[~2023-04-10  2:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-10  2:21 Lu Wei [this message]
2023-04-10 13:02 ` [PATCH net] net: Add check for csum_start in skb_partial_csum_set() Eric Dumazet
2023-04-10 17:30   ` Willem de Bruijn
     [not found]     ` <450994d7-4a77-99df-6317-b535ea73e01d@huawei.com>
2023-04-11  8:13       ` Eric Dumazet
2023-04-11 13:50         ` luwei (O)
2023-04-11 14:03         ` luwei (O)
2023-04-12 13:44           ` Willem de Bruijn
2023-04-14  8:45             ` 答复: " luwei (O)
2023-04-14 16:48               ` Willem de Bruijn
2023-04-14 17:20                 ` Willem de Bruijn
2023-04-15  2:18                   ` luwei (O)
2023-04-15 13:47                     ` Willem de Bruijn
2023-04-18  1:59                       ` luwei (O)
2023-04-18 12:44                         ` Willem de Bruijn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230410022152.4049060-1-luwei32@huawei.com \
    --to=luwei32@huawei.com \
    --cc=asml.silence@gmail.com \
    --cc=brouer@redhat.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=imagedong@tencent.com \
    --cc=jbenc@redhat.com \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).