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

2016-01-04 Thread Eric Dumazet
On Mon, 2016-01-04 at 23:08 -0500, David Miller wrote:
> From: Eric Dumazet 
> Date: Wed, 30 Dec 2015 13:05:45 -0500
> 
> > On Thu, 2015-09-24 at 14:09 -0700, David Miller wrote:
> >> From: Pravin B Shelar 
> >> 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 
> >> > Signed-off-by: Pravin B Shelar 
> >> 
> >> 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;




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


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

2016-01-04 Thread David Miller
From: Eric Dumazet 
Date: Wed, 30 Dec 2015 13:05:45 -0500

> On Thu, 2015-09-24 at 14:09 -0700, David Miller wrote:
>> From: Pravin B Shelar 
>> 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 
>> > Signed-off-by: Pravin B Shelar 
>> 
>> 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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-12-30 Thread Eric Dumazet
On Thu, 2015-09-24 at 14:09 -0700, David Miller wrote:
> From: Pravin B Shelar 
> 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 
> > Signed-off-by: Pravin B Shelar 
> 
> 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);



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


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

2015-09-28 Thread David Miller
From: Pravin Shelar 
Date: Fri, 25 Sep 2015 19:48:57 -0700

> On Thu, Sep 24, 2015 at 2:09 PM, David Miller  wrote:
>> From: Pravin B Shelar 
>> 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 
>>> Signed-off-by: Pravin B Shelar 
>>
>> 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.

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


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

2015-09-25 Thread Pravin Shelar
On Thu, Sep 24, 2015 at 2:09 PM, David Miller  wrote:
> From: Pravin B Shelar 
> 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 
>> Signed-off-by: Pravin B Shelar 
>
> Applied, thanks.

Thanks for applying it. Since I have seen this bug on older kernel can
you also queue it for -stable.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-09-24 Thread David Miller
From: Pravin B Shelar 
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 
> Signed-off-by: Pravin B Shelar 

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


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

2015-09-22 Thread Tom Herbert
On Tue, Sep 22, 2015 at 12:57 PM, Pravin B Shelar  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:[] skb_checksum_help+0x144/0x150
> Call Trace:
> 
> [] queue_userspace_packet+0x408/0x470 [openvswitch]
> [] ovs_dp_upcall+0x5d/0x60 [openvswitch]
> [] ovs_dp_process_packet_with_key+0xe6/0x100 [openvswitch]
> [] ovs_dp_process_received_packet+0x4b/0x80 [openvswitch]
> [] ovs_vport_receive+0x2a/0x30 [openvswitch]
> [] vxlan_rcv+0x53/0x60 [openvswitch]
> [] vxlan_udp_encap_recv+0x8b/0xf0 [openvswitch]
> [] udp_queue_rcv_skb+0x2dc/0x3b0
> [] __udp4_lib_rcv+0x1cf/0x6c0
> [] udp_rcv+0x1a/0x20
> [] ip_local_deliver_finish+0xdd/0x280
> [] ip_local_deliver+0x88/0x90
> [] ip_rcv_finish+0x10d/0x370
> [] ip_rcv+0x235/0x300
> [] __netif_receive_skb+0x55d/0x620
> [] netif_receive_skb+0x80/0x90
> [] virtnet_poll+0x555/0x6f0
> [] net_rx_action+0x134/0x290
> [] __do_softirq+0xa8/0x210
> [] call_softirq+0x1c/0x30
> [] do_softirq+0x65/0xa0
> [] irq_exit+0x8e/0xb0
> [] do_IRQ+0x63/0xe0
> [] common_interrupt+0x6e/0x6e
>
> Reported-by: Anupam Chanda 
> Signed-off-by: Pravin B Shelar 
> ---
> v1-v2:
> Set skb to CHECKSUM_NONE rather than CHECKSUM_UNNECESSARY.
> ---
>  include/linux/skbuff.h |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> 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;
>  }
>
Acked-by: Tom Herbert 

>  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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-09-22 Thread Pravin B Shelar
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:[] skb_checksum_help+0x144/0x150
Call Trace:

[] queue_userspace_packet+0x408/0x470 [openvswitch]
[] ovs_dp_upcall+0x5d/0x60 [openvswitch]
[] ovs_dp_process_packet_with_key+0xe6/0x100 [openvswitch]
[] ovs_dp_process_received_packet+0x4b/0x80 [openvswitch]
[] ovs_vport_receive+0x2a/0x30 [openvswitch]
[] vxlan_rcv+0x53/0x60 [openvswitch]
[] vxlan_udp_encap_recv+0x8b/0xf0 [openvswitch]
[] udp_queue_rcv_skb+0x2dc/0x3b0
[] __udp4_lib_rcv+0x1cf/0x6c0
[] udp_rcv+0x1a/0x20
[] ip_local_deliver_finish+0xdd/0x280
[] ip_local_deliver+0x88/0x90
[] ip_rcv_finish+0x10d/0x370
[] ip_rcv+0x235/0x300
[] __netif_receive_skb+0x55d/0x620
[] netif_receive_skb+0x80/0x90
[] virtnet_poll+0x555/0x6f0
[] net_rx_action+0x134/0x290
[] __do_softirq+0xa8/0x210
[] call_softirq+0x1c/0x30
[] do_softirq+0x65/0xa0
[] irq_exit+0x8e/0xb0
[] do_IRQ+0x63/0xe0
[] common_interrupt+0x6e/0x6e

Reported-by: Anupam Chanda 
Signed-off-by: Pravin B Shelar 
---
v1-v2:
Set skb to CHECKSUM_NONE rather than CHECKSUM_UNNECESSARY.
---
 include/linux/skbuff.h |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

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

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