[PATCH net v2] skbuff: Fix skb checksum flag on skb pull

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

------------[ cut here ]------------
kernel BUG at net/core/dev.c:1906!
RIP: 0010:[<ffffffff81518034>] skb_checksum_help+0x144/0x150
Call Trace:
<IRQ>
[<ffffffffa0164c28>] queue_userspace_packet+0x408/0x470 [openvswitch]
[<ffffffffa016614d>] ovs_dp_upcall+0x5d/0x60 [openvswitch]
[<ffffffffa0166236>] ovs_dp_process_packet_with_key+0xe6/0x100 [openvswitch]
[<ffffffffa016629b>] ovs_dp_process_received_packet+0x4b/0x80 [openvswitch]
[<ffffffffa016c51a>] ovs_vport_receive+0x2a/0x30 [openvswitch]
[<ffffffffa0171383>] vxlan_rcv+0x53/0x60 [openvswitch]
[<ffffffffa01734cb>] vxlan_udp_encap_recv+0x8b/0xf0 [openvswitch]
[<ffffffff8157addc>] udp_queue_rcv_skb+0x2dc/0x3b0
[<ffffffff8157b56f>] __udp4_lib_rcv+0x1cf/0x6c0
[<ffffffff8157ba7a>] udp_rcv+0x1a/0x20
[<ffffffff8154fdbd>] ip_local_deliver_finish+0xdd/0x280
[<ffffffff81550128>] ip_local_deliver+0x88/0x90
[<ffffffff8154fa7d>] ip_rcv_finish+0x10d/0x370
[<ffffffff81550365>] ip_rcv+0x235/0x300
[<ffffffff8151ba1d>] __netif_receive_skb+0x55d/0x620
[<ffffffff8151c360>] netif_receive_skb+0x80/0x90
[<ffffffff81459935>] virtnet_poll+0x555/0x6f0
[<ffffffff8151cd04>] net_rx_action+0x134/0x290
[<ffffffff810683d8>] __do_softirq+0xa8/0x210
[<ffffffff8162fe6c>] call_softirq+0x1c/0x30
[<ffffffff810161a5>] do_softirq+0x65/0xa0
[<ffffffff810687be>] irq_exit+0x8e/0xb0
[<ffffffff81630733>] do_IRQ+0x63/0xe0
[<ffffffff81625f2e>] common_interrupt+0x6e/0x6e

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

7 more lines

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 9987af0..2b0a30a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h

@@ -2707,6 +2707,9 @@ static inline void skb_postpull_rcsum(struct sk_buff *skb,
 {
 	if (skb->ip_summed == CHECKSUM_COMPLETE)
 		skb->csum = csum_sub(skb->csum, csum_partial(start, len, 0));
+	else if (skb->ip_summed == CHECKSUM_PARTIAL &&
+		 skb_checksum_start_offset(skb) <= len)
+		skb->ip_summed = CHECKSUM_NONE;
 }
 
 unsigned char *skb_pull_rcsum(struct sk_buff *skb, unsigned int len);

-- 
1.7.1

On Tue, Sep 22, 2015 at 12:57 PM, Pravin B Shelar [off-list ref] wrote:

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

------------[ cut here ]------------
kernel BUG at net/core/dev.c:1906!
RIP: 0010:[<ffffffff81518034>] skb_checksum_help+0x144/0x150
Call Trace:
<IRQ>
[<ffffffffa0164c28>] queue_userspace_packet+0x408/0x470 [openvswitch]
[<ffffffffa016614d>] ovs_dp_upcall+0x5d/0x60 [openvswitch]
[<ffffffffa0166236>] ovs_dp_process_packet_with_key+0xe6/0x100 [openvswitch]
[<ffffffffa016629b>] ovs_dp_process_received_packet+0x4b/0x80 [openvswitch]
[<ffffffffa016c51a>] ovs_vport_receive+0x2a/0x30 [openvswitch]
[<ffffffffa0171383>] vxlan_rcv+0x53/0x60 [openvswitch]
[<ffffffffa01734cb>] vxlan_udp_encap_recv+0x8b/0xf0 [openvswitch]
[<ffffffff8157addc>] udp_queue_rcv_skb+0x2dc/0x3b0
[<ffffffff8157b56f>] __udp4_lib_rcv+0x1cf/0x6c0
[<ffffffff8157ba7a>] udp_rcv+0x1a/0x20
[<ffffffff8154fdbd>] ip_local_deliver_finish+0xdd/0x280
[<ffffffff81550128>] ip_local_deliver+0x88/0x90
[<ffffffff8154fa7d>] ip_rcv_finish+0x10d/0x370
[<ffffffff81550365>] ip_rcv+0x235/0x300
[<ffffffff8151ba1d>] __netif_receive_skb+0x55d/0x620
[<ffffffff8151c360>] netif_receive_skb+0x80/0x90
[<ffffffff81459935>] virtnet_poll+0x555/0x6f0
[<ffffffff8151cd04>] net_rx_action+0x134/0x290
[<ffffffff810683d8>] __do_softirq+0xa8/0x210
[<ffffffff8162fe6c>] call_softirq+0x1c/0x30
[<ffffffff810161a5>] do_softirq+0x65/0xa0
[<ffffffff810687be>] irq_exit+0x8e/0xb0
[<ffffffff81630733>] do_IRQ+0x63/0xe0
[<ffffffff81625f2e>] common_interrupt+0x6e/0x6e

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

20 more quoted lines

Acked-by: Tom Herbert <redacted>

 unsigned char *skb_pull_rcsum(struct sk_buff *skb, unsigned int len);
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Pravin B Shelar <redacted>
Date: Tue, 22 Sep 2015 12:57:53 -0700

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

 ...

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

Applied, thanks.

On Thu, Sep 24, 2015 at 2:09 PM, David Miller [off-list ref] wrote:

From: Pravin B Shelar <redacted>
Date: Tue, 22 Sep 2015 12:57:53 -0700

quoted

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

 ...

quoted

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

Applied, thanks.

Thanks for applying it. Since I have seen this bug on older kernel can
you also queue it for -stable.

From: Pravin Shelar <redacted>
Date: Fri, 25 Sep 2015 19:48:57 -0700

On Thu, Sep 24, 2015 at 2:09 PM, David Miller [off-list ref] wrote:

quoted

From: Pravin B Shelar <redacted>
Date: Tue, 22 Sep 2015 12:57:53 -0700

quoted

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

 ...

quoted

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

Applied, thanks.

Thanks for applying it. Since I have seen this bug on older kernel can
you also queue it for -stable.

Not until we resolve all of the regressions caused by it.

On Thu, 2015-09-24 at 14:09 -0700, David Miller wrote:

From: Pravin B Shelar <redacted>
Date: Tue, 22 Sep 2015 12:57:53 -0700

quoted

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

 ...

quoted

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

Applied, thanks.

It looks like we also should clear skb->csum ?

__skb_checksum_complete() definitely would be confused by garbage in
skb->csum

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6b6bd42d6134..43e6f6163e07 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h

@@ -2799,8 +2799,10 @@ static inline void skb_postpull_rcsum(struct sk_buff *skb,
 	if (skb->ip_summed == CHECKSUM_COMPLETE)
 		skb->csum = csum_sub(skb->csum, csum_partial(start, len, 0));
 	else if (skb->ip_summed == CHECKSUM_PARTIAL &&
-		 skb_checksum_start_offset(skb) < 0)
+		 skb_checksum_start_offset(skb) < 0) {
 		skb->ip_summed = CHECKSUM_NONE;
+		skb->csum = 0;
+	}
 }
 
 unsigned char *skb_pull_rcsum(struct sk_buff *skb, unsigned int len);

From: Eric Dumazet <redacted>
Date: Wed, 30 Dec 2015 13:05:45 -0500

On Thu, 2015-09-24 at 14:09 -0700, David Miller wrote:

quoted

From: Pravin B Shelar <redacted>
Date: Tue, 22 Sep 2015 12:57:53 -0700

quoted

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

 ...

quoted

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

Applied, thanks.

It looks like we also should clear skb->csum ?

__skb_checksum_complete() definitely would be confused by garbage in
skb->csum

Pravin, please take a look at this.

On Mon, 2016-01-04 at 23:08 -0500, David Miller wrote:

From: Eric Dumazet <redacted>
Date: Wed, 30 Dec 2015 13:05:45 -0500

quoted

On Thu, 2015-09-24 at 14:09 -0700, David Miller wrote:

quoted

From: Pravin B Shelar <redacted>
Date: Tue, 22 Sep 2015 12:57:53 -0700

quoted

VXLAN device can receive skb with checksum partial. But the checksum
offset could be in outer header which is pulled on receive. This results
in negative checksum offset for the skb. Such skb can cause the assert
failure in skb_checksum_help(). Following patch fixes the bug by setting
checksum-none while pulling outer header.

Following is the kernel panic msg from old kernel hitting the bug.

 ...

quoted

Reported-by: Anupam Chanda <redacted>
Signed-off-by: Pravin B Shelar <redacted>

Applied, thanks.

It looks like we also should clear skb->csum ?

__skb_checksum_complete() definitely would be confused by garbage in
skb->csum

Pravin, please take a look at this.

This might be ok :

Apparently users are supposed to init skb->csum in this case, as in 

udp4_csum_init()
-> skb_checksum_init_zero_check()
  -> __skb_checksum_validate()

-> __skb_checksum_validate_complete
   skb->csum = psum;