netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH,v2] ipvlan: add the check of ip header checksum
@ 2020-07-22  9:18 guodeqing
  2020-07-22 16:29 ` Eric Dumazet
  2020-07-22 20:04 ` David Miller
  0 siblings, 2 replies; 10+ messages in thread
From: guodeqing @ 2020-07-22  9:18 UTC (permalink / raw)
  To: davem; +Cc: kuba, maheshb, edumazet, netdev, geffrey.guo

The ip header checksum can be error in the following steps.
$ ip netns add ns1
$ ip link add gw link eth0 type ipvlan
$ ip addr add 168.16.0.1/24 dev gw
$ ip link set dev gw up
$ ip link add ip1 link eth0 type ipvlan
$ ip link set ip1 netns ns1
$ ip netns exec ns1 ip link set ip1 up
$ ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1
$ ip netns exec ns1 tc qdisc add dev ip1 root netem corrupt 50%
$ ip netns exec ns1 ping 168.16.0.1

This is because the netem will modify the packet randomly. the
corrupted packets should be dropped derectly, otherwise it may
cause a problem.

Here I add the check of ip header checksum and drop the illegal
packets in l3/l3s mode.

Signed-off-by: guodeqing <geffrey.guo@huawei.com>
---
 drivers/net/ipvlan/ipvlan_core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 8801d09..ccf3193 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -160,6 +160,10 @@ void *ipvlan_get_L3_hdr(struct ipvl_port *port, struct sk_buff *skb, int *type)
 		pktlen = ntohs(ip4h->tot_len);
 		if (ip4h->ihl < 5 || ip4h->version != 4)
 			return NULL;
+
+		if (unlikely(ip_fast_csum((u8 *)ip4h, ip4h->ihl)))
+			return NULL;
+
 		if (skb->len < pktlen || pktlen < (ip4h->ihl * 4))
 			return NULL;
 
@@ -569,8 +573,10 @@ static int ipvlan_xmit_mode_l3(struct sk_buff *skb, struct net_device *dev)
 	int addr_type;
 
 	lyr3h = ipvlan_get_L3_hdr(ipvlan->port, skb, &addr_type);
-	if (!lyr3h)
-		goto out;
+	if (!lyr3h) {
+		kfree_skb(skb);
+		return NET_XMIT_DROP;
+	}
 
 	if (!ipvlan_is_vepa(ipvlan->port)) {
 		addr = ipvlan_addr_lookup(ipvlan->port, lyr3h, addr_type, true);
@@ -582,7 +588,7 @@ static int ipvlan_xmit_mode_l3(struct sk_buff *skb, struct net_device *dev)
 			return ipvlan_rcv_frame(addr, &skb, true);
 		}
 	}
-out:
+
 	ipvlan_skb_crossing_ns(skb, ipvlan->phy_dev);
 	return ipvlan_process_outbound(skb);
 }
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-22  9:18 [PATCH,v2] ipvlan: add the check of ip header checksum guodeqing
@ 2020-07-22 16:29 ` Eric Dumazet
  2020-07-23  2:02   ` 答复: " Guodeqing (A)
  2020-07-22 20:04 ` David Miller
  1 sibling, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2020-07-22 16:29 UTC (permalink / raw)
  To: guodeqing; +Cc: David Miller, Jakub Kicinski, Mahesh Bandewar, netdev

On Wed, Jul 22, 2020 at 2:23 AM guodeqing <geffrey.guo@huawei.com> wrote:
>
> The ip header checksum can be error in the following steps.
> $ ip netns add ns1
> $ ip link add gw link eth0 type ipvlan
> $ ip addr add 168.16.0.1/24 dev gw
> $ ip link set dev gw up
> $ ip link add ip1 link eth0 type ipvlan
> $ ip link set ip1 netns ns1
> $ ip netns exec ns1 ip link set ip1 up
> $ ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1
> $ ip netns exec ns1 tc qdisc add dev ip1 root netem corrupt 50%
> $ ip netns exec ns1 ping 168.16.0.1
>
> This is because the netem will modify the packet randomly. the
> corrupted packets should be dropped derectly, otherwise it may
> cause a problem.


And why would ipvlan be so special ?

What about all other drivers ?

My advice : Do not use netem corrupt if you do not want to send
corrupted packets .

>
>
> Here I add the check of ip header checksum and drop the illegal
> packets in l3/l3s mode.
>

This patch makes no sense really.

>
> Signed-off-by: guodeqing <geffrey.guo@huawei.com>
> ---

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-22  9:18 [PATCH,v2] ipvlan: add the check of ip header checksum guodeqing
  2020-07-22 16:29 ` Eric Dumazet
@ 2020-07-22 20:04 ` David Miller
  2020-07-23  1:59   ` 答复: " Guodeqing (A)
  1 sibling, 1 reply; 10+ messages in thread
From: David Miller @ 2020-07-22 20:04 UTC (permalink / raw)
  To: geffrey.guo; +Cc: kuba, maheshb, edumazet, netdev

From: guodeqing <geffrey.guo@huawei.com>
Date: Wed, 22 Jul 2020 17:18:19 +0800

> The ip header checksum can be error in the following steps.

You did not respond to my feedback from your previous submissions.

Packets created inside of the kernel have correct checksums, and
the ipvlan driver can depend upon this precondition.

I am not applying this patch, sorry.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* 答复: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-22 20:04 ` David Miller
@ 2020-07-23  1:59   ` Guodeqing (A)
  2020-07-23  2:15     ` Eric Dumazet
  0 siblings, 1 reply; 10+ messages in thread
From: Guodeqing (A) @ 2020-07-23  1:59 UTC (permalink / raw)
  To: David Miller; +Cc: kuba, maheshb, edumazet, netdev

I am sorry, the mail is not sent to you directly; 

If do the following test,this will cause a panic in a arm64 VM. this can be reproduced easily.

Linux osc 5.8.0-rc6+ #3 SMP Thu Jul 23 01:40:47 UTC 2020 aarch64

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
root@osc:~# ifconfig eth0 up
root@osc:~# ip netns add ns1
root@osc:~# ip link add gw link eth0 type ipvlan
root@osc:~# ip addr add 168.16.0.1/24 dev gw
root@osc:~# ip link set dev gw up
root@osc:~# ip link add ip1 link eth0 type ipvlan
root@osc:~# ip link set ip1 netns ns1
root@osc:~# ip netns exec ns1 ip link set ip1 up
root@osc:~# ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1
root@osc:~# ip netns exec ns1 ip link set lo up
root@osc:~# ip netns exec ns1 ip addr add 127.0.0.1/8 dev lo
RTNETLINK answers: File exists
root@osc:~# ip netns exec ns1 tc qdisc add dev ip1 root netem corrupt 100%
root@osc:~# ip netns exec ns1 ping 168.16.0.1
PING 168.16.0.1 (168.16.0.1) 56(84) bytes of data.
From 168.16.0.1 icmp_seq=2 Destination Host Unreachable
From 168.16.0.1 icmp_seq=12 Destination Host Unreachable
From 168.16.0.1 icmp_seq=30 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=48 ttl=64 time=0.052 ms
64 bytes from 168.16.0.1: icmp_seq=65 ttl=64 time=0.060 ms
From 168.16.0.1 icmp_seq=80 Destination Host Unreachable
From 168.16.0.1 icmp_seq=97 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=100 ttl=64 time=0.022 ms
64 bytes from 168.16.0.1: icmp_seq=101 ttl=64 time=0.054 ms
64 bytes from 168.16.0.1: icmp_seq=103 ttl=64 time=0.053 ms
From 168.16.0.1 icmp_seq=102 Destination Host Unreachable
From 168.16.0.1 icmp_seq=127 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=132 ttl=64 time=0.057 ms
64 bytes from 168.16.0.1: icmp_seq=135 ttl=64 time=0.051 ms
64 bytes from 168.16.0.1: icmp_seq=141 ttl=64 time=0.051 ms
From 168.16.0.1 icmp_seq=140 Destination Host Unreachable
From 168.16.0.1 icmp_seq=142 Destination Host Unreachable
From 168.16.0.1 icmp_seq=150 Destination Host Unreachable
From 168.16.0.1 icmp_seq=154 Destination Host Unreachable
From 168.16.0.1 icmp_seq=164 Destination Host Unreachable
From 168.16.0.1 icmp_seq=169 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=173 ttl=64 time=0.056 ms
64 bytes from 168.16.0.1: icmp_seq=185 ttl=64 time=0.058 ms
64 bytes from 168.16.0.1: icmp_seq=202 ttl=64 time=0.056 ms
64 bytes from 168.16.0.1: icmp_seq=203 ttl=64 time=0.057 ms
64 bytes from 168.16.0.1: icmp_seq=219 ttl=64 time=0.050 ms
64 bytes from 168.16.0.1: icmp_seq=227 ttl=64 time=0.057 ms
64 bytes from 168.16.0.1: icmp_seq=228 ttl=64 time=0.044 ms
From 168.16.0.1 icmp_seq=237 Destination Host Unreachable
From 168.16.0.1 icmp_seq=239 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=243 ttl=64 time=0.053 ms
From 168.16.0.1 icmp_seq=242 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=246 ttl=64 time=0.056 ms
64 bytes from 168.16.0.1: icmp_seq=254 ttl=64 time=0.056 ms
64 bytes from 168.16.0.1: icmp_seq=255 ttl=64 time=0.054 ms
From 168.16.0.1 icmp_seq=263 Destination Host Unreachable
From 168.16.0.1 icmp_seq=269 Destination Host Unreachable
From 168.16.0.1 icmp_seq=273 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=279 ttl=64 time=0.057 ms
From 168.16.0.1 icmp_seq=284 Destination Host Unreachable
From 168.16.0.1 icmp_seq=286 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=291 ttl=64 time=0.054 ms
64 bytes from 168.16.0.1: icmp_seq=293 ttl=64 time=0.058 ms
From 168.16.0.1 icmp_seq=294 Destination Host Unreachable
From 168.16.0.1 icmp_seq=298 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=309 ttl=64 time=0.056 ms
From 168.16.0.1 icmp_seq=310 Destination Host Unreachable
From 168.16.0.1 icmp_seq=312 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=332 ttl=64 time=0.055 ms
From 168.16.0.1 icmp_seq=334 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=340 ttl=64 time=0.055 ms
64 bytes from 168.16.0.1: icmp_seq=344 ttl=64 time=0.050 ms
64 bytes from 168.16.0.1: icmp_seq=355 ttl=64 time=0.057 ms
64 bytes from 168.16.0.1: icmp_seq=358 ttl=64 time=0.050 ms
From 168.16.0.1 icmp_seq=356 Destination Host Unreachable
From 168.16.0.1 icmp_seq=369 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=382 ttl=64 time=0.053 ms
From 168.16.0.1 icmp_seq=381 Destination Host Unreachable
From 168.16.0.1 icmp_seq=396 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=400 ttl=64 time=0.058 ms
From 168.16.0.1 icmp_seq=406 Destination Host Unreachable
From 168.16.0.1 icmp_seq=414 Destination Host Unreachable
From 168.16.0.1 icmp_seq=417 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=422 ttl=64 time=0.018 ms
64 bytes from 168.16.0.1: icmp_seq=423 ttl=64 time=0.058 ms
64 bytes from 168.16.0.1: icmp_seq=429 ttl=64 time=0.056 ms
64 bytes from 168.16.0.1: icmp_seq=445 ttl=64 time=0.049 ms
From 168.16.0.1 icmp_seq=444 Destination Host Unreachable
From 168.16.0.1 icmp_seq=453 Destination Host Unreachable
From 168.16.0.1 icmp_seq=455 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=456 ttl=64 time=0.024 ms
64 bytes from 168.16.0.1: icmp_seq=469 ttl=64 time=0.058 ms
From 168.16.0.1 icmp_seq=475 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=483 ttl=64 time=0.054 ms
From 168.16.0.1 icmp_seq=488 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=504 ttl=64 time=0.056 ms
64 bytes from 168.16.0.1: icmp_seq=505 ttl=64 time=0.055 ms
From 168.16.0.1 icmp_seq=510 Destination Host Unreachable
From 168.16.0.1 icmp_seq=511 Destination Host Unreachable
64 bytes from 168.16.0.1: icmp_seq=516 ttl=64 time=0.055 ms
From 168.16.0.1 icmp_seq=519 Destination Host Unreachable
[  582.368938] Unable to handle kernel paging request at virtual address ffff0000f85f0000
[  582.369732] Mem abort info:
[  582.369987]   ESR = 0x96000007
[  582.370266]   EC = 0x25: DABT (current EL), IL = 32 bits
[  582.370833]   SET = 0, FnV = 0
[  582.371113]   EA = 0, S1PTW = 0
[  582.371391] Data abort info:
[  582.371671]   ISV = 0, ISS = 0x00000007
[  582.372017]   CM = 0, WnR = 0
[  582.372299] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000012dab7000
[  582.372896] [ffff0000f85f0000] pgd=000000013fff8003, p4d=000000013fff8003, pud=000000013f9f4003, pmd=000000013f838003, pte=0000000000000000
[  582.374033] Internal error: Oops: 96000007 [#1] SMP
[  582.374468] Modules linked in:
[  582.374795] CPU: 1 PID: 525 Comm: ping Not tainted 5.8.0-rc6+ #3
[  582.375468] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[  582.376215] pstate: 20400005 (nzCv daif +PAN -UAO BTYPE=--)
[  582.376805] pc : __ip_local_out+0x84/0x188
[  582.377234] lr : ip_local_out+0x34/0x68
[  582.377635] sp : ffff800013263440
[  582.377986] x29: ffff800013263440 x28: 0000000000000001
[  582.378536] x27: ffff8000111d2018 x26: ffff8000114cba80
[  582.379093] x25: ffff0000ec4e7400 x24: 0000000000000000
[  582.379653] x23: 0000000000000062 x22: ffff8000114c9000
[  582.380221] x21: ffff0000d97ac600 x20: ffff0000ec519000
[  582.380778] x19: ffff8000115b5bc0 x18: 0000000000000000
[  582.381324] x17: 0000000000000000 x16: 0000000000000000
[  582.381876] x15: 0000000000000000 x14: 0000000000000000
[  582.382431] x13: 0000000000000000 x12: 0000000000000001
[  582.382986] x11: ffff800010d21838 x10: 0000000000000001
[  582.383567] x9 : 0000000000000001 x8 : 0000000000000000
[  582.384136] x7 : 0000000000000000 x6 : ffff0000ec4e5e00
[  582.384693] x5 : 024079ca54000184 x4 : ffff0000ec4e5e10
[  582.385246] x3 : 0000000000000000 x2 : ffff0004ec4e5e20
[  582.385808] x1 : ffff0000f85f0000 x0 : 031d079626a9c7ae
[  582.386365] Call trace:
[  582.386629]  __ip_local_out+0x84/0x188
[  582.387030]  ip_local_out+0x34/0x68
[  582.387400]  ipvlan_queue_xmit+0x548/0x5c0
[  582.387845]  ipvlan_start_xmit+0x2c/0x90
[  582.388283]  dev_hard_start_xmit+0xb4/0x260
[  582.388732]  sch_direct_xmit+0x1b4/0x550
[  582.389145]  __qdisc_run+0x140/0x648
[  582.389524]  __dev_queue_xmit+0x6a4/0x8b8
[  582.389948]  dev_queue_xmit+0x24/0x30
[  582.390339]  ip_finish_output2+0x324/0x580
[  582.390770]  __ip_finish_output+0x130/0x218
[  582.391218]  ip_finish_output+0x38/0xd0
[  582.391633]  ip_output+0xb4/0x130
[  582.391984]  ip_local_out+0x58/0x68
[  582.392369]  ip_send_skb+0x2c/0x88
[  582.392729]  ip_push_pending_frames+0x44/0x50
[  582.393189]  raw_sendmsg+0x7a4/0x988
[  582.393569]  inet_sendmsg+0x4c/0x78
[  582.393942]  sock_sendmsg+0x58/0x68
[  582.394311]  ____sys_sendmsg+0x284/0x2c0
[  582.394721]  ___sys_sendmsg+0x90/0xd0
[  582.395113]  __sys_sendmsg+0x78/0xd0
[  582.395504]  __arm64_sys_sendmsg+0x2c/0x38
[  582.395942]  el0_svc_common.constprop.2+0x70/0x128
[  582.396472]  do_el0_svc+0x34/0xa0
[  582.396834]  el0_sync_handler+0xec/0x128
[  582.397249]  el0_sync+0x140/0x180
[  582.397611] Code: ab030005 91001442 9a030000 8b020882 (b8404423)
[  582.398264] ---[ end trace 92adb54c8611f8c5 ]---
[  582.398754] Kernel panic - not syncing: Fatal exception in interrupt
[  582.399481] SMP: stopping secondary CPUs
[  582.399923] Kernel Offset: 0xc0000 from 0xffff800010000000
[  582.400561] PHYS_OFFSET: 0x40000000
[  582.400939] CPU features: 0x040002,22a08238
[  582.401380] Memory Limit: none
[  582.401710] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

This panic is because the ip header is corrupted. The ihl of the ip header is error,this cause ip_fast_csum access the illegal address.
23 static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
 24 {
 25     __uint128_t tmp;
 26     u64 sum;
 27
 28     tmp = *(const __uint128_t *)iph;
 29     iph += 16;
 30     ihl -= 4;                                           -----here, if ihl is smaller than 5, the next will access the illegal address.
 31     tmp += ((tmp >> 64) | (tmp << 64));
 32     sum = tmp >> 64;
 33     do {
 34         sum += *(const u32 *)iph;
 35         iph += 4;
 36     } while (--ihl);
 37
 38     sum += ((sum >> 32) | (sum << 32));
 39     return csum_fold((__force u32)(sum >> 32));
 40 }

I think this panic may be a problem, thanks.



-----邮件原件-----
发件人: Cong Wang [mailto:xiyou.wangcong@gmail.com]
发送时间: Wednesday, July 22, 2020 3:39
收件人: Guodeqing (A) <geffrey.guo@huawei.com>
抄送: David Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mahesh Bandewar <maheshb@google.com>; Eric Dumazet <edumazet@google.com>; Linux Kernel Network Developers <netdev@vger.kernel.org>
主题: Re: [PATCH] ipvlan: add the check of ip header checksum

On Tue, Jul 21, 2020 at 6:17 AM guodeqing <geffrey.guo@huawei.com> wrote:
>
> The ip header checksum can be error in the following steps.
> $ ip netns add ns1
> $ ip link add gw link eth0 type ipvlan $ ip addr add 168.16.0.1/24 dev 
> gw $ ip link set dev gw up $ ip link add ip1 link eth0 type ipvlan $ 
> ip link set ip1 netns ns1 $ ip netns exec ns1 ip link set ip1 up $ ip 
> netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 $ ip netns exec ns1 
> tc qdisc add dev ip1 root netem corrupt 50% $ ip netns exec ns1 ping
> 168.16.0.1
>
> The ip header of a packet maybe modified when it steps in 
> ipvlan_process_v4_outbound because of the netem, the corruptted 
> packets should be dropped.

This does not make much sense, as you intentionally corrupt the header. More importantly, the check you add is too late, right?
ipvlan_xmit_mode_l3() already does the addr lookup with IP header,

Thanks.

-----邮件原件-----
发件人: David Miller [mailto:davem@davemloft.net] 
发送时间: Thursday, July 23, 2020 4:04
收件人: Guodeqing (A) <geffrey.guo@huawei.com>
抄送: kuba@kernel.org; maheshb@google.com; edumazet@google.com; netdev@vger.kernel.org
主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum

From: guodeqing <geffrey.guo@huawei.com>
Date: Wed, 22 Jul 2020 17:18:19 +0800

> The ip header checksum can be error in the following steps.

You did not respond to my feedback from your previous submissions.

Packets created inside of the kernel have correct checksums, and the ipvlan driver can depend upon this precondition.

I am not applying this patch, sorry.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* 答复: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-22 16:29 ` Eric Dumazet
@ 2020-07-23  2:02   ` Guodeqing (A)
  0 siblings, 0 replies; 10+ messages in thread
From: Guodeqing (A) @ 2020-07-23  2:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Jakub Kicinski, Mahesh Bandewar, netdev

Because ipvlan will do the ip forward, it will do a route lookup.if the ihl of the ip header is smaller than 5, this cause ip_fast_csum access the illegal address.

-----邮件原件-----
发件人: Eric Dumazet [mailto:edumazet@google.com] 
发送时间: Thursday, July 23, 2020 0:30
收件人: Guodeqing (A) <geffrey.guo@huawei.com>
抄送: David Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mahesh Bandewar <maheshb@google.com>; netdev <netdev@vger.kernel.org>
主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum

On Wed, Jul 22, 2020 at 2:23 AM guodeqing <geffrey.guo@huawei.com> wrote:
>
> The ip header checksum can be error in the following steps.
> $ ip netns add ns1
> $ ip link add gw link eth0 type ipvlan $ ip addr add 168.16.0.1/24 dev 
> gw $ ip link set dev gw up $ ip link add ip1 link eth0 type ipvlan $ 
> ip link set ip1 netns ns1 $ ip netns exec ns1 ip link set ip1 up $ ip 
> netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 $ ip netns exec ns1 
> tc qdisc add dev ip1 root netem corrupt 50% $ ip netns exec ns1 ping 
> 168.16.0.1
>
> This is because the netem will modify the packet randomly. the 
> corrupted packets should be dropped derectly, otherwise it may cause a 
> problem.


And why would ipvlan be so special ?

What about all other drivers ?

My advice : Do not use netem corrupt if you do not want to send corrupted packets .

>
>
> Here I add the check of ip header checksum and drop the illegal 
> packets in l3/l3s mode.
>

This patch makes no sense really.

>
> Signed-off-by: guodeqing <geffrey.guo@huawei.com>
> ---

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-23  1:59   ` 答复: " Guodeqing (A)
@ 2020-07-23  2:15     ` Eric Dumazet
  2020-07-24  3:35       ` 答复: " Guodeqing (A)
  0 siblings, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2020-07-23  2:15 UTC (permalink / raw)
  To: Guodeqing (A); +Cc: David Miller, kuba, maheshb, netdev

On Wed, Jul 22, 2020 at 6:59 PM Guodeqing (A) <geffrey.guo@huawei.com> wrote:
>
> I am sorry, the mail is not sent to you directly;
>
> If do the following test,this will cause a panic in a arm64 VM. this can be reproduced easily.
>
> Linux osc 5.8.0-rc6+ #3 SMP Thu Jul 23 01:40:47 UTC 2020 aarch64
>
> The programs included with the Debian GNU/Linux system are free software;
> the exact distribution terms for each program are described in the
> individual files in /usr/share/doc/*/copyright.
>
> Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
> permitted by applicable law.
> root@osc:~# ifconfig eth0 up
> root@osc:~# ip netns add ns1
> root@osc:~# ip link add gw link eth0 type ipvlan
> root@osc:~# ip addr add 168.16.0.1/24 dev gw
> root@osc:~# ip link set dev gw up
> root@osc:~# ip link add ip1 link eth0 type ipvlan
> root@osc:~# ip link set ip1 netns ns1
> root@osc:~# ip netns exec ns1 ip link set ip1 up
> root@osc:~# ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1
> root@osc:~# ip netns exec ns1 ip link set lo up
> root@osc:~# ip netns exec ns1 ip addr add 127.0.0.1/8 dev lo
> RTNETLINK answers: File exists
> root@osc:~# ip netns exec ns1 tc qdisc add dev ip1 root netem corrupt 100%
> root@osc:~# ip netns exec ns1 ping 168.16.0.1
> PING 168.16.0.1 (168.16.0.1) 56(84) bytes of data.
> From 168.16.0.1 icmp_seq=2 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=12 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=30 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=48 ttl=64 time=0.052 ms
> 64 bytes from 168.16.0.1: icmp_seq=65 ttl=64 time=0.060 ms
> From 168.16.0.1 icmp_seq=80 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=97 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=100 ttl=64 time=0.022 ms
> 64 bytes from 168.16.0.1: icmp_seq=101 ttl=64 time=0.054 ms
> 64 bytes from 168.16.0.1: icmp_seq=103 ttl=64 time=0.053 ms
> From 168.16.0.1 icmp_seq=102 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=127 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=132 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=135 ttl=64 time=0.051 ms
> 64 bytes from 168.16.0.1: icmp_seq=141 ttl=64 time=0.051 ms
> From 168.16.0.1 icmp_seq=140 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=142 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=150 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=154 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=164 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=169 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=173 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=185 ttl=64 time=0.058 ms
> 64 bytes from 168.16.0.1: icmp_seq=202 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=203 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=219 ttl=64 time=0.050 ms
> 64 bytes from 168.16.0.1: icmp_seq=227 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=228 ttl=64 time=0.044 ms
> From 168.16.0.1 icmp_seq=237 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=239 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=243 ttl=64 time=0.053 ms
> From 168.16.0.1 icmp_seq=242 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=246 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=254 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=255 ttl=64 time=0.054 ms
> From 168.16.0.1 icmp_seq=263 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=269 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=273 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=279 ttl=64 time=0.057 ms
> From 168.16.0.1 icmp_seq=284 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=286 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=291 ttl=64 time=0.054 ms
> 64 bytes from 168.16.0.1: icmp_seq=293 ttl=64 time=0.058 ms
> From 168.16.0.1 icmp_seq=294 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=298 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=309 ttl=64 time=0.056 ms
> From 168.16.0.1 icmp_seq=310 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=312 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=332 ttl=64 time=0.055 ms
> From 168.16.0.1 icmp_seq=334 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=340 ttl=64 time=0.055 ms
> 64 bytes from 168.16.0.1: icmp_seq=344 ttl=64 time=0.050 ms
> 64 bytes from 168.16.0.1: icmp_seq=355 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=358 ttl=64 time=0.050 ms
> From 168.16.0.1 icmp_seq=356 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=369 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=382 ttl=64 time=0.053 ms
> From 168.16.0.1 icmp_seq=381 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=396 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=400 ttl=64 time=0.058 ms
> From 168.16.0.1 icmp_seq=406 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=414 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=417 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=422 ttl=64 time=0.018 ms
> 64 bytes from 168.16.0.1: icmp_seq=423 ttl=64 time=0.058 ms
> 64 bytes from 168.16.0.1: icmp_seq=429 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=445 ttl=64 time=0.049 ms
> From 168.16.0.1 icmp_seq=444 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=453 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=455 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=456 ttl=64 time=0.024 ms
> 64 bytes from 168.16.0.1: icmp_seq=469 ttl=64 time=0.058 ms
> From 168.16.0.1 icmp_seq=475 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=483 ttl=64 time=0.054 ms
> From 168.16.0.1 icmp_seq=488 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=504 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=505 ttl=64 time=0.055 ms
> From 168.16.0.1 icmp_seq=510 Destination Host Unreachable
> From 168.16.0.1 icmp_seq=511 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=516 ttl=64 time=0.055 ms
> From 168.16.0.1 icmp_seq=519 Destination Host Unreachable
> [  582.368938] Unable to handle kernel paging request at virtual address ffff0000f85f0000
> [  582.369732] Mem abort info:
> [  582.369987]   ESR = 0x96000007
> [  582.370266]   EC = 0x25: DABT (current EL), IL = 32 bits
> [  582.370833]   SET = 0, FnV = 0
> [  582.371113]   EA = 0, S1PTW = 0
> [  582.371391] Data abort info:
> [  582.371671]   ISV = 0, ISS = 0x00000007
> [  582.372017]   CM = 0, WnR = 0
> [  582.372299] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000012dab7000
> [  582.372896] [ffff0000f85f0000] pgd=000000013fff8003, p4d=000000013fff8003, pud=000000013f9f4003, pmd=000000013f838003, pte=0000000000000000
> [  582.374033] Internal error: Oops: 96000007 [#1] SMP
> [  582.374468] Modules linked in:
> [  582.374795] CPU: 1 PID: 525 Comm: ping Not tainted 5.8.0-rc6+ #3
> [  582.375468] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> [  582.376215] pstate: 20400005 (nzCv daif +PAN -UAO BTYPE=--)
> [  582.376805] pc : __ip_local_out+0x84/0x188
> [  582.377234] lr : ip_local_out+0x34/0x68
> [  582.377635] sp : ffff800013263440
> [  582.377986] x29: ffff800013263440 x28: 0000000000000001
> [  582.378536] x27: ffff8000111d2018 x26: ffff8000114cba80
> [  582.379093] x25: ffff0000ec4e7400 x24: 0000000000000000
> [  582.379653] x23: 0000000000000062 x22: ffff8000114c9000
> [  582.380221] x21: ffff0000d97ac600 x20: ffff0000ec519000
> [  582.380778] x19: ffff8000115b5bc0 x18: 0000000000000000
> [  582.381324] x17: 0000000000000000 x16: 0000000000000000
> [  582.381876] x15: 0000000000000000 x14: 0000000000000000
> [  582.382431] x13: 0000000000000000 x12: 0000000000000001
> [  582.382986] x11: ffff800010d21838 x10: 0000000000000001
> [  582.383567] x9 : 0000000000000001 x8 : 0000000000000000
> [  582.384136] x7 : 0000000000000000 x6 : ffff0000ec4e5e00
> [  582.384693] x5 : 024079ca54000184 x4 : ffff0000ec4e5e10
> [  582.385246] x3 : 0000000000000000 x2 : ffff0004ec4e5e20
> [  582.385808] x1 : ffff0000f85f0000 x0 : 031d079626a9c7ae
> [  582.386365] Call trace:
> [  582.386629]  __ip_local_out+0x84/0x188
> [  582.387030]  ip_local_out+0x34/0x68
> [  582.387400]  ipvlan_queue_xmit+0x548/0x5c0
> [  582.387845]  ipvlan_start_xmit+0x2c/0x90
> [  582.388283]  dev_hard_start_xmit+0xb4/0x260
> [  582.388732]  sch_direct_xmit+0x1b4/0x550
> [  582.389145]  __qdisc_run+0x140/0x648
> [  582.389524]  __dev_queue_xmit+0x6a4/0x8b8
> [  582.389948]  dev_queue_xmit+0x24/0x30
> [  582.390339]  ip_finish_output2+0x324/0x580
> [  582.390770]  __ip_finish_output+0x130/0x218
> [  582.391218]  ip_finish_output+0x38/0xd0
> [  582.391633]  ip_output+0xb4/0x130
> [  582.391984]  ip_local_out+0x58/0x68
> [  582.392369]  ip_send_skb+0x2c/0x88
> [  582.392729]  ip_push_pending_frames+0x44/0x50
> [  582.393189]  raw_sendmsg+0x7a4/0x988
> [  582.393569]  inet_sendmsg+0x4c/0x78
> [  582.393942]  sock_sendmsg+0x58/0x68
> [  582.394311]  ____sys_sendmsg+0x284/0x2c0
> [  582.394721]  ___sys_sendmsg+0x90/0xd0
> [  582.395113]  __sys_sendmsg+0x78/0xd0
> [  582.395504]  __arm64_sys_sendmsg+0x2c/0x38
> [  582.395942]  el0_svc_common.constprop.2+0x70/0x128
> [  582.396472]  do_el0_svc+0x34/0xa0
> [  582.396834]  el0_sync_handler+0xec/0x128
> [  582.397249]  el0_sync+0x140/0x180
> [  582.397611] Code: ab030005 91001442 9a030000 8b020882 (b8404423)
> [  582.398264] ---[ end trace 92adb54c8611f8c5 ]---
> [  582.398754] Kernel panic - not syncing: Fatal exception in interrupt
> [  582.399481] SMP: stopping secondary CPUs
> [  582.399923] Kernel Offset: 0xc0000 from 0xffff800010000000
> [  582.400561] PHYS_OFFSET: 0x40000000
> [  582.400939] CPU features: 0x040002,22a08238
> [  582.401380] Memory Limit: none
> [  582.401710] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---
>
> This panic is because the ip header is corrupted. The ihl of the ip header is error,this cause ip_fast_csum access the illegal address.
> 23 static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
>  24 {
>  25     __uint128_t tmp;
>  26     u64 sum;
>  27
>  28     tmp = *(const __uint128_t *)iph;
>  29     iph += 16;
>  30     ihl -= 4;                                           -----here, if ihl is smaller than 5, the next will access the illegal address.


Well, the bug is there then.

Code for other arches is different.

x86 for instance has a test for silly ihl

static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
{
unsigned int sum;

asm("  movl (%1), %0\n"
    "  subl $4, %2\n"
    "  jbe 2f\n"                          --- here
    "  addl 4(%1), %0\n"
    "  adcl 8(%1), %0\n"
    "  adcl 12(%1), %0\n"
    "1: adcl 16(%1), %0\n"
    "  lea 4(%1), %1\n"
    "  decl %2\n"
    "  jne 1b\n"
    "  adcl $0, %0\n"
    "  movl %0, %2\n"
    "  shrl $16, %0\n"
    "  addw %w2, %w0\n"
    "  adcl $0, %0\n"
    "  notl %0\n"
    "2:"
/* Since the input registers which are loaded with iph and ihl
   are modified, we must also specify them as outputs, or gcc
   will assume they contain their original values. */
    : "=r" (sum), "=r" (iph), "=r" (ihl)
    : "1" (iph), "2" (ihl)
    : "memory");
return (__force __sum16)sum;
}


>  31     tmp += ((tmp >> 64) | (tmp << 64));
>  32     sum = tmp >> 64;
>  33     do {
>  34         sum += *(const u32 *)iph;
>  35         iph += 4;
>  36     } while (--ihl);
>  37
>  38     sum += ((sum >> 32) | (sum << 32));
>  39     return csum_fold((__force u32)(sum >> 32));
>  40 }
>
> I think this panic may be a problem, thanks.
>
>
>
> -----邮件原件-----
> 发件人: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> 发送时间: Wednesday, July 22, 2020 3:39
> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> 抄送: David Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Mahesh Bandewar <maheshb@google.com>; Eric Dumazet <edumazet@google.com>; Linux Kernel Network Developers <netdev@vger.kernel.org>
> 主题: Re: [PATCH] ipvlan: add the check of ip header checksum
>
> On Tue, Jul 21, 2020 at 6:17 AM guodeqing <geffrey.guo@huawei.com> wrote:
> >
> > The ip header checksum can be error in the following steps.
> > $ ip netns add ns1
> > $ ip link add gw link eth0 type ipvlan $ ip addr add 168.16.0.1/24 dev
> > gw $ ip link set dev gw up $ ip link add ip1 link eth0 type ipvlan $
> > ip link set ip1 netns ns1 $ ip netns exec ns1 ip link set ip1 up $ ip
> > netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 $ ip netns exec ns1
> > tc qdisc add dev ip1 root netem corrupt 50% $ ip netns exec ns1 ping
> > 168.16.0.1
> >
> > The ip header of a packet maybe modified when it steps in
> > ipvlan_process_v4_outbound because of the netem, the corruptted
> > packets should be dropped.
>
> This does not make much sense, as you intentionally corrupt the header. More importantly, the check you add is too late, right?
> ipvlan_xmit_mode_l3() already does the addr lookup with IP header,
>
> Thanks.
>
> -----邮件原件-----
> 发件人: David Miller [mailto:davem@davemloft.net]
> 发送时间: Thursday, July 23, 2020 4:04
> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> 抄送: kuba@kernel.org; maheshb@google.com; edumazet@google.com; netdev@vger.kernel.org
> 主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum
>
> From: guodeqing <geffrey.guo@huawei.com>
> Date: Wed, 22 Jul 2020 17:18:19 +0800
>
> > The ip header checksum can be error in the following steps.
>
> You did not respond to my feedback from your previous submissions.
>
> Packets created inside of the kernel have correct checksums, and the ipvlan driver can depend upon this precondition.
>
> I am not applying this patch, sorry.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* 答复: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-23  2:15     ` Eric Dumazet
@ 2020-07-24  3:35       ` Guodeqing (A)
  2020-07-24  5:13         ` Eric Dumazet
  2020-07-24 23:44         ` David Miller
  0 siblings, 2 replies; 10+ messages in thread
From: Guodeqing (A) @ 2020-07-24  3:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, kuba, maheshb, netdev

The ihl check maybe not suitable in ip_fast_csum, the correct of the ihl value can be checked before calling the ip_fast_csum.

The implementation of ip_fast_csum is different in different cpu architecture. the IP packet will do ip forward in the ipvlan l3/l3s mode and the corrupted ip packet

should be discarded as soon as possible. Thanks.


-----邮件原件-----
发件人: Eric Dumazet [mailto:edumazet@google.com] 
发送时间: Thursday, July 23, 2020 10:15
收件人: Guodeqing (A) <geffrey.guo@huawei.com>
抄送: David Miller <davem@davemloft.net>; kuba@kernel.org; maheshb@google.com; netdev@vger.kernel.org
主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum

On Wed, Jul 22, 2020 at 6:59 PM Guodeqing (A) <geffrey.guo@huawei.com> wrote:
>
> I am sorry, the mail is not sent to you directly;
>
> If do the following test,this will cause a panic in a arm64 VM. this can be reproduced easily.
>
> Linux osc 5.8.0-rc6+ #3 SMP Thu Jul 23 01:40:47 UTC 2020 aarch64
>
> The programs included with the Debian GNU/Linux system are free 
> software; the exact distribution terms for each program are described 
> in the individual files in /usr/share/doc/*/copyright.
>
> Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent 
> permitted by applicable law.
> root@osc:~# ifconfig eth0 up
> root@osc:~# ip netns add ns1
> root@osc:~# ip link add gw link eth0 type ipvlan root@osc:~# ip addr 
> add 168.16.0.1/24 dev gw root@osc:~# ip link set dev gw up root@osc:~# 
> ip link add ip1 link eth0 type ipvlan root@osc:~# ip link set ip1 
> netns ns1 root@osc:~# ip netns exec ns1 ip link set ip1 up root@osc:~# 
> ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 root@osc:~# ip 
> netns exec ns1 ip link set lo up root@osc:~# ip netns exec ns1 ip addr 
> add 127.0.0.1/8 dev lo RTNETLINK answers: File exists root@osc:~# ip 
> netns exec ns1 tc qdisc add dev ip1 root netem corrupt 100% 
> root@osc:~# ip netns exec ns1 ping 168.16.0.1 PING 168.16.0.1 
> (168.16.0.1) 56(84) bytes of data.
> From 168.16.0.1 icmp_seq=2 Destination Host Unreachable From 
> 168.16.0.1 icmp_seq=12 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=30 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=48 ttl=64 time=0.052 ms
> 64 bytes from 168.16.0.1: icmp_seq=65 ttl=64 time=0.060 ms From 
> 168.16.0.1 icmp_seq=80 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=97 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=100 ttl=64 time=0.022 ms
> 64 bytes from 168.16.0.1: icmp_seq=101 ttl=64 time=0.054 ms
> 64 bytes from 168.16.0.1: icmp_seq=103 ttl=64 time=0.053 ms From 
> 168.16.0.1 icmp_seq=102 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=127 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=132 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=135 ttl=64 time=0.051 ms
> 64 bytes from 168.16.0.1: icmp_seq=141 ttl=64 time=0.051 ms From 
> 168.16.0.1 icmp_seq=140 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=142 Destination Host Unreachable From 168.16.0.1 icmp_seq=150 
> Destination Host Unreachable From 168.16.0.1 icmp_seq=154 Destination 
> Host Unreachable From 168.16.0.1 icmp_seq=164 Destination Host 
> Unreachable From 168.16.0.1 icmp_seq=169 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=173 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=185 ttl=64 time=0.058 ms
> 64 bytes from 168.16.0.1: icmp_seq=202 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=203 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=219 ttl=64 time=0.050 ms
> 64 bytes from 168.16.0.1: icmp_seq=227 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=228 ttl=64 time=0.044 ms From 
> 168.16.0.1 icmp_seq=237 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=239 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=243 ttl=64 time=0.053 ms From 
> 168.16.0.1 icmp_seq=242 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=246 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=254 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=255 ttl=64 time=0.054 ms From 
> 168.16.0.1 icmp_seq=263 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=269 Destination Host Unreachable From 168.16.0.1 icmp_seq=273 
> Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=279 ttl=64 time=0.057 ms From 
> 168.16.0.1 icmp_seq=284 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=286 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=291 ttl=64 time=0.054 ms
> 64 bytes from 168.16.0.1: icmp_seq=293 ttl=64 time=0.058 ms From 
> 168.16.0.1 icmp_seq=294 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=298 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=309 ttl=64 time=0.056 ms From 
> 168.16.0.1 icmp_seq=310 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=312 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=332 ttl=64 time=0.055 ms From 
> 168.16.0.1 icmp_seq=334 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=340 ttl=64 time=0.055 ms
> 64 bytes from 168.16.0.1: icmp_seq=344 ttl=64 time=0.050 ms
> 64 bytes from 168.16.0.1: icmp_seq=355 ttl=64 time=0.057 ms
> 64 bytes from 168.16.0.1: icmp_seq=358 ttl=64 time=0.050 ms From 
> 168.16.0.1 icmp_seq=356 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=369 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=382 ttl=64 time=0.053 ms From 
> 168.16.0.1 icmp_seq=381 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=396 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=400 ttl=64 time=0.058 ms From 
> 168.16.0.1 icmp_seq=406 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=414 Destination Host Unreachable From 168.16.0.1 icmp_seq=417 
> Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=422 ttl=64 time=0.018 ms
> 64 bytes from 168.16.0.1: icmp_seq=423 ttl=64 time=0.058 ms
> 64 bytes from 168.16.0.1: icmp_seq=429 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=445 ttl=64 time=0.049 ms From 
> 168.16.0.1 icmp_seq=444 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=453 Destination Host Unreachable From 168.16.0.1 icmp_seq=455 
> Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=456 ttl=64 time=0.024 ms
> 64 bytes from 168.16.0.1: icmp_seq=469 ttl=64 time=0.058 ms From 
> 168.16.0.1 icmp_seq=475 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=483 ttl=64 time=0.054 ms From 
> 168.16.0.1 icmp_seq=488 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=504 ttl=64 time=0.056 ms
> 64 bytes from 168.16.0.1: icmp_seq=505 ttl=64 time=0.055 ms From 
> 168.16.0.1 icmp_seq=510 Destination Host Unreachable From 168.16.0.1 
> icmp_seq=511 Destination Host Unreachable
> 64 bytes from 168.16.0.1: icmp_seq=516 ttl=64 time=0.055 ms From 
> 168.16.0.1 icmp_seq=519 Destination Host Unreachable [  582.368938] 
> Unable to handle kernel paging request at virtual address 
> ffff0000f85f0000 [  582.369732] Mem abort info:
> [  582.369987]   ESR = 0x96000007
> [  582.370266]   EC = 0x25: DABT (current EL), IL = 32 bits
> [  582.370833]   SET = 0, FnV = 0
> [  582.371113]   EA = 0, S1PTW = 0
> [  582.371391] Data abort info:
> [  582.371671]   ISV = 0, ISS = 0x00000007
> [  582.372017]   CM = 0, WnR = 0
> [  582.372299] swapper pgtable: 4k pages, 48-bit VAs, 
> pgdp=000000012dab7000 [  582.372896] [ffff0000f85f0000] 
> pgd=000000013fff8003, p4d=000000013fff8003, pud=000000013f9f4003, 
> pmd=000000013f838003, pte=0000000000000000 [  582.374033] Internal error: Oops: 96000007 [#1] SMP [  582.374468] Modules linked in:
> [  582.374795] CPU: 1 PID: 525 Comm: ping Not tainted 5.8.0-rc6+ #3 [  
> 582.375468] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 
> 02/06/2015 [  582.376215] pstate: 20400005 (nzCv daif +PAN -UAO 
> BTYPE=--) [  582.376805] pc : __ip_local_out+0x84/0x188 [  582.377234] 
> lr : ip_local_out+0x34/0x68 [  582.377635] sp : ffff800013263440 [  
> 582.377986] x29: ffff800013263440 x28: 0000000000000001 [  582.378536] 
> x27: ffff8000111d2018 x26: ffff8000114cba80 [  582.379093] x25: 
> ffff0000ec4e7400 x24: 0000000000000000 [  582.379653] x23: 
> 0000000000000062 x22: ffff8000114c9000 [  582.380221] x21: 
> ffff0000d97ac600 x20: ffff0000ec519000 [  582.380778] x19: 
> ffff8000115b5bc0 x18: 0000000000000000 [  582.381324] x17: 
> 0000000000000000 x16: 0000000000000000 [  582.381876] x15: 
> 0000000000000000 x14: 0000000000000000 [  582.382431] x13: 
> 0000000000000000 x12: 0000000000000001 [  582.382986] x11: 
> ffff800010d21838 x10: 0000000000000001 [  582.383567] x9 : 
> 0000000000000001 x8 : 0000000000000000 [  582.384136] x7 : 
> 0000000000000000 x6 : ffff0000ec4e5e00 [  582.384693] x5 : 
> 024079ca54000184 x4 : ffff0000ec4e5e10 [  582.385246] x3 : 
> 0000000000000000 x2 : ffff0004ec4e5e20 [  582.385808] x1 : 
> ffff0000f85f0000 x0 : 031d079626a9c7ae [  582.386365] Call trace:
> [  582.386629]  __ip_local_out+0x84/0x188 [  582.387030]  
> ip_local_out+0x34/0x68 [  582.387400]  ipvlan_queue_xmit+0x548/0x5c0 [  
> 582.387845]  ipvlan_start_xmit+0x2c/0x90 [  582.388283]  
> dev_hard_start_xmit+0xb4/0x260 [  582.388732]  
> sch_direct_xmit+0x1b4/0x550 [  582.389145]  __qdisc_run+0x140/0x648 [  
> 582.389524]  __dev_queue_xmit+0x6a4/0x8b8 [  582.389948]  
> dev_queue_xmit+0x24/0x30 [  582.390339]  ip_finish_output2+0x324/0x580 
> [  582.390770]  __ip_finish_output+0x130/0x218 [  582.391218]  
> ip_finish_output+0x38/0xd0 [  582.391633]  ip_output+0xb4/0x130 [  
> 582.391984]  ip_local_out+0x58/0x68 [  582.392369]  
> ip_send_skb+0x2c/0x88 [  582.392729]  ip_push_pending_frames+0x44/0x50 
> [  582.393189]  raw_sendmsg+0x7a4/0x988 [  582.393569]  
> inet_sendmsg+0x4c/0x78 [  582.393942]  sock_sendmsg+0x58/0x68 [  
> 582.394311]  ____sys_sendmsg+0x284/0x2c0 [  582.394721]  
> ___sys_sendmsg+0x90/0xd0 [  582.395113]  __sys_sendmsg+0x78/0xd0 [  
> 582.395504]  __arm64_sys_sendmsg+0x2c/0x38 [  582.395942]  
> el0_svc_common.constprop.2+0x70/0x128
> [  582.396472]  do_el0_svc+0x34/0xa0
> [  582.396834]  el0_sync_handler+0xec/0x128 [  582.397249]  
> el0_sync+0x140/0x180 [  582.397611] Code: ab030005 91001442 9a030000 
> 8b020882 (b8404423) [  582.398264] ---[ end trace 92adb54c8611f8c5 
> ]--- [  582.398754] Kernel panic - not syncing: Fatal exception in 
> interrupt [  582.399481] SMP: stopping secondary CPUs [  582.399923] 
> Kernel Offset: 0xc0000 from 0xffff800010000000 [  582.400561] 
> PHYS_OFFSET: 0x40000000 [  582.400939] CPU features: 0x040002,22a08238 
> [  582.401380] Memory Limit: none [  582.401710] ---[ end Kernel panic 
> - not syncing: Fatal exception in interrupt ]---
>
> This panic is because the ip header is corrupted. The ihl of the ip header is error,this cause ip_fast_csum access the illegal address.
> 23 static inline __sum16 ip_fast_csum(const void *iph, unsigned int 
> ihl)
>  24 {
>  25     __uint128_t tmp;
>  26     u64 sum;
>  27
>  28     tmp = *(const __uint128_t *)iph;
>  29     iph += 16;
>  30     ihl -= 4;                                           -----here, if ihl is smaller than 5, the next will access the illegal address.


Well, the bug is there then.

Code for other arches is different.

x86 for instance has a test for silly ihl

static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl) { unsigned int sum;

asm("  movl (%1), %0\n"
    "  subl $4, %2\n"
    "  jbe 2f\n"                          --- here
    "  addl 4(%1), %0\n"
    "  adcl 8(%1), %0\n"
    "  adcl 12(%1), %0\n"
    "1: adcl 16(%1), %0\n"
    "  lea 4(%1), %1\n"
    "  decl %2\n"
    "  jne 1b\n"
    "  adcl $0, %0\n"
    "  movl %0, %2\n"
    "  shrl $16, %0\n"
    "  addw %w2, %w0\n"
    "  adcl $0, %0\n"
    "  notl %0\n"
    "2:"
/* Since the input registers which are loaded with iph and ihl
   are modified, we must also specify them as outputs, or gcc
   will assume they contain their original values. */
    : "=r" (sum), "=r" (iph), "=r" (ihl)
    : "1" (iph), "2" (ihl)
    : "memory");
return (__force __sum16)sum;
}


>  31     tmp += ((tmp >> 64) | (tmp << 64));
>  32     sum = tmp >> 64;
>  33     do {
>  34         sum += *(const u32 *)iph;
>  35         iph += 4;
>  36     } while (--ihl);
>  37
>  38     sum += ((sum >> 32) | (sum << 32));
>  39     return csum_fold((__force u32)(sum >> 32));
>  40 }
>
> I think this panic may be a problem, thanks.
>
>
>
> -----邮件原件-----
> 发件人: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> 发送时间: Wednesday, July 22, 2020 3:39
> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> 抄送: David Miller <davem@davemloft.net>; Jakub Kicinski 
> <kuba@kernel.org>; Mahesh Bandewar <maheshb@google.com>; Eric Dumazet 
> <edumazet@google.com>; Linux Kernel Network Developers 
> <netdev@vger.kernel.org>
> 主题: Re: [PATCH] ipvlan: add the check of ip header checksum
>
> On Tue, Jul 21, 2020 at 6:17 AM guodeqing <geffrey.guo@huawei.com> wrote:
> >
> > The ip header checksum can be error in the following steps.
> > $ ip netns add ns1
> > $ ip link add gw link eth0 type ipvlan $ ip addr add 168.16.0.1/24 
> > dev gw $ ip link set dev gw up $ ip link add ip1 link eth0 type 
> > ipvlan $ ip link set ip1 netns ns1 $ ip netns exec ns1 ip link set 
> > ip1 up $ ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 $ ip 
> > netns exec ns1 tc qdisc add dev ip1 root netem corrupt 50% $ ip 
> > netns exec ns1 ping
> > 168.16.0.1
> >
> > The ip header of a packet maybe modified when it steps in 
> > ipvlan_process_v4_outbound because of the netem, the corruptted 
> > packets should be dropped.
>
> This does not make much sense, as you intentionally corrupt the header. More importantly, the check you add is too late, right?
> ipvlan_xmit_mode_l3() already does the addr lookup with IP header,
>
> Thanks.
>
> -----邮件原件-----
> 发件人: David Miller [mailto:davem@davemloft.net]
> 发送时间: Thursday, July 23, 2020 4:04
> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> 抄送: kuba@kernel.org; maheshb@google.com; edumazet@google.com; 
> netdev@vger.kernel.org
> 主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum
>
> From: guodeqing <geffrey.guo@huawei.com>
> Date: Wed, 22 Jul 2020 17:18:19 +0800
>
> > The ip header checksum can be error in the following steps.
>
> You did not respond to my feedback from your previous submissions.
>
> Packets created inside of the kernel have correct checksums, and the ipvlan driver can depend upon this precondition.
>
> I am not applying this patch, sorry.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: 答复: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-24  3:35       ` 答复: " Guodeqing (A)
@ 2020-07-24  5:13         ` Eric Dumazet
  2020-07-24  8:39           ` 答复: " Guodeqing (A)
  2020-07-24 23:44         ` David Miller
  1 sibling, 1 reply; 10+ messages in thread
From: Eric Dumazet @ 2020-07-24  5:13 UTC (permalink / raw)
  To: Guodeqing (A), Eric Dumazet; +Cc: David Miller, kuba, maheshb, netdev

Please do not top-post on netdev / lkml  ( https://www.mediawiki.org/wiki/Mailing_list_etiquette )



On 7/23/20 8:35 PM, Guodeqing (A) wrote:
> The ihl check maybe not suitable in ip_fast_csum, the correct of the ihl value can be checked before calling the ip_fast_csum.
> 
> The implementation of ip_fast_csum is different in different cpu architecture. the IP packet will do ip forward in the ipvlan l3/l3s mode and the corrupted ip packet
> 
> should be discarded as soon as possible. Thanks.

Then the "as soon as possible" should be done in netem, right ?

Alternatively fix arm64 ip_fast_csum() so that it matches the reference implementation (lib/checksum.c)

We do not want adding code in ipvlan that is essentially dead code for normal usages.

ip_fast_csum() is not free.

So, this is a NACK from my side, in case this was not clear enough.



> 
> 
> -----邮件原件-----
> 发件人: Eric Dumazet [mailto:edumazet@google.com] 
> 发送时间: Thursday, July 23, 2020 10:15
> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> 抄送: David Miller <davem@davemloft.net>; kuba@kernel.org; maheshb@google.com; netdev@vger.kernel.org
> 主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum
> 
> On Wed, Jul 22, 2020 at 6:59 PM Guodeqing (A) <geffrey.guo@huawei.com> wrote:
>>
>> I am sorry, the mail is not sent to you directly;
>>
>> If do the following test,this will cause a panic in a arm64 VM. this can be reproduced easily.
>>
>> Linux osc 5.8.0-rc6+ #3 SMP Thu Jul 23 01:40:47 UTC 2020 aarch64
>>
>> The programs included with the Debian GNU/Linux system are free 
>> software; the exact distribution terms for each program are described 
>> in the individual files in /usr/share/doc/*/copyright.
>>
>> Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent 
>> permitted by applicable law.
>> root@osc:~# ifconfig eth0 up
>> root@osc:~# ip netns add ns1
>> root@osc:~# ip link add gw link eth0 type ipvlan root@osc:~# ip addr 
>> add 168.16.0.1/24 dev gw root@osc:~# ip link set dev gw up root@osc:~# 
>> ip link add ip1 link eth0 type ipvlan root@osc:~# ip link set ip1 
>> netns ns1 root@osc:~# ip netns exec ns1 ip link set ip1 up root@osc:~# 
>> ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 root@osc:~# ip 
>> netns exec ns1 ip link set lo up root@osc:~# ip netns exec ns1 ip addr 
>> add 127.0.0.1/8 dev lo RTNETLINK answers: File exists root@osc:~# ip 
>> netns exec ns1 tc qdisc add dev ip1 root netem corrupt 100% 
>> root@osc:~# ip netns exec ns1 ping 168.16.0.1 PING 168.16.0.1 
>> (168.16.0.1) 56(84) bytes of data.
>> From 168.16.0.1 icmp_seq=2 Destination Host Unreachable From 
>> 168.16.0.1 icmp_seq=12 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=30 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=48 ttl=64 time=0.052 ms
>> 64 bytes from 168.16.0.1: icmp_seq=65 ttl=64 time=0.060 ms From 
>> 168.16.0.1 icmp_seq=80 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=97 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=100 ttl=64 time=0.022 ms
>> 64 bytes from 168.16.0.1: icmp_seq=101 ttl=64 time=0.054 ms
>> 64 bytes from 168.16.0.1: icmp_seq=103 ttl=64 time=0.053 ms From 
>> 168.16.0.1 icmp_seq=102 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=127 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=132 ttl=64 time=0.057 ms
>> 64 bytes from 168.16.0.1: icmp_seq=135 ttl=64 time=0.051 ms
>> 64 bytes from 168.16.0.1: icmp_seq=141 ttl=64 time=0.051 ms From 
>> 168.16.0.1 icmp_seq=140 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=142 Destination Host Unreachable From 168.16.0.1 icmp_seq=150 
>> Destination Host Unreachable From 168.16.0.1 icmp_seq=154 Destination 
>> Host Unreachable From 168.16.0.1 icmp_seq=164 Destination Host 
>> Unreachable From 168.16.0.1 icmp_seq=169 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=173 ttl=64 time=0.056 ms
>> 64 bytes from 168.16.0.1: icmp_seq=185 ttl=64 time=0.058 ms
>> 64 bytes from 168.16.0.1: icmp_seq=202 ttl=64 time=0.056 ms
>> 64 bytes from 168.16.0.1: icmp_seq=203 ttl=64 time=0.057 ms
>> 64 bytes from 168.16.0.1: icmp_seq=219 ttl=64 time=0.050 ms
>> 64 bytes from 168.16.0.1: icmp_seq=227 ttl=64 time=0.057 ms
>> 64 bytes from 168.16.0.1: icmp_seq=228 ttl=64 time=0.044 ms From 
>> 168.16.0.1 icmp_seq=237 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=239 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=243 ttl=64 time=0.053 ms From 
>> 168.16.0.1 icmp_seq=242 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=246 ttl=64 time=0.056 ms
>> 64 bytes from 168.16.0.1: icmp_seq=254 ttl=64 time=0.056 ms
>> 64 bytes from 168.16.0.1: icmp_seq=255 ttl=64 time=0.054 ms From 
>> 168.16.0.1 icmp_seq=263 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=269 Destination Host Unreachable From 168.16.0.1 icmp_seq=273 
>> Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=279 ttl=64 time=0.057 ms From 
>> 168.16.0.1 icmp_seq=284 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=286 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=291 ttl=64 time=0.054 ms
>> 64 bytes from 168.16.0.1: icmp_seq=293 ttl=64 time=0.058 ms From 
>> 168.16.0.1 icmp_seq=294 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=298 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=309 ttl=64 time=0.056 ms From 
>> 168.16.0.1 icmp_seq=310 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=312 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=332 ttl=64 time=0.055 ms From 
>> 168.16.0.1 icmp_seq=334 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=340 ttl=64 time=0.055 ms
>> 64 bytes from 168.16.0.1: icmp_seq=344 ttl=64 time=0.050 ms
>> 64 bytes from 168.16.0.1: icmp_seq=355 ttl=64 time=0.057 ms
>> 64 bytes from 168.16.0.1: icmp_seq=358 ttl=64 time=0.050 ms From 
>> 168.16.0.1 icmp_seq=356 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=369 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=382 ttl=64 time=0.053 ms From 
>> 168.16.0.1 icmp_seq=381 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=396 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=400 ttl=64 time=0.058 ms From 
>> 168.16.0.1 icmp_seq=406 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=414 Destination Host Unreachable From 168.16.0.1 icmp_seq=417 
>> Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=422 ttl=64 time=0.018 ms
>> 64 bytes from 168.16.0.1: icmp_seq=423 ttl=64 time=0.058 ms
>> 64 bytes from 168.16.0.1: icmp_seq=429 ttl=64 time=0.056 ms
>> 64 bytes from 168.16.0.1: icmp_seq=445 ttl=64 time=0.049 ms From 
>> 168.16.0.1 icmp_seq=444 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=453 Destination Host Unreachable From 168.16.0.1 icmp_seq=455 
>> Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=456 ttl=64 time=0.024 ms
>> 64 bytes from 168.16.0.1: icmp_seq=469 ttl=64 time=0.058 ms From 
>> 168.16.0.1 icmp_seq=475 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=483 ttl=64 time=0.054 ms From 
>> 168.16.0.1 icmp_seq=488 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=504 ttl=64 time=0.056 ms
>> 64 bytes from 168.16.0.1: icmp_seq=505 ttl=64 time=0.055 ms From 
>> 168.16.0.1 icmp_seq=510 Destination Host Unreachable From 168.16.0.1 
>> icmp_seq=511 Destination Host Unreachable
>> 64 bytes from 168.16.0.1: icmp_seq=516 ttl=64 time=0.055 ms From 
>> 168.16.0.1 icmp_seq=519 Destination Host Unreachable [  582.368938] 
>> Unable to handle kernel paging request at virtual address 
>> ffff0000f85f0000 [  582.369732] Mem abort info:
>> [  582.369987]   ESR = 0x96000007
>> [  582.370266]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [  582.370833]   SET = 0, FnV = 0
>> [  582.371113]   EA = 0, S1PTW = 0
>> [  582.371391] Data abort info:
>> [  582.371671]   ISV = 0, ISS = 0x00000007
>> [  582.372017]   CM = 0, WnR = 0
>> [  582.372299] swapper pgtable: 4k pages, 48-bit VAs, 
>> pgdp=000000012dab7000 [  582.372896] [ffff0000f85f0000] 
>> pgd=000000013fff8003, p4d=000000013fff8003, pud=000000013f9f4003, 
>> pmd=000000013f838003, pte=0000000000000000 [  582.374033] Internal error: Oops: 96000007 [#1] SMP [  582.374468] Modules linked in:
>> [  582.374795] CPU: 1 PID: 525 Comm: ping Not tainted 5.8.0-rc6+ #3 [  
>> 582.375468] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 
>> 02/06/2015 [  582.376215] pstate: 20400005 (nzCv daif +PAN -UAO 
>> BTYPE=--) [  582.376805] pc : __ip_local_out+0x84/0x188 [  582.377234] 
>> lr : ip_local_out+0x34/0x68 [  582.377635] sp : ffff800013263440 [  
>> 582.377986] x29: ffff800013263440 x28: 0000000000000001 [  582.378536] 
>> x27: ffff8000111d2018 x26: ffff8000114cba80 [  582.379093] x25: 
>> ffff0000ec4e7400 x24: 0000000000000000 [  582.379653] x23: 
>> 0000000000000062 x22: ffff8000114c9000 [  582.380221] x21: 
>> ffff0000d97ac600 x20: ffff0000ec519000 [  582.380778] x19: 
>> ffff8000115b5bc0 x18: 0000000000000000 [  582.381324] x17: 
>> 0000000000000000 x16: 0000000000000000 [  582.381876] x15: 
>> 0000000000000000 x14: 0000000000000000 [  582.382431] x13: 
>> 0000000000000000 x12: 0000000000000001 [  582.382986] x11: 
>> ffff800010d21838 x10: 0000000000000001 [  582.383567] x9 : 
>> 0000000000000001 x8 : 0000000000000000 [  582.384136] x7 : 
>> 0000000000000000 x6 : ffff0000ec4e5e00 [  582.384693] x5 : 
>> 024079ca54000184 x4 : ffff0000ec4e5e10 [  582.385246] x3 : 
>> 0000000000000000 x2 : ffff0004ec4e5e20 [  582.385808] x1 : 
>> ffff0000f85f0000 x0 : 031d079626a9c7ae [  582.386365] Call trace:
>> [  582.386629]  __ip_local_out+0x84/0x188 [  582.387030]  
>> ip_local_out+0x34/0x68 [  582.387400]  ipvlan_queue_xmit+0x548/0x5c0 [  
>> 582.387845]  ipvlan_start_xmit+0x2c/0x90 [  582.388283]  
>> dev_hard_start_xmit+0xb4/0x260 [  582.388732]  
>> sch_direct_xmit+0x1b4/0x550 [  582.389145]  __qdisc_run+0x140/0x648 [  
>> 582.389524]  __dev_queue_xmit+0x6a4/0x8b8 [  582.389948]  
>> dev_queue_xmit+0x24/0x30 [  582.390339]  ip_finish_output2+0x324/0x580 
>> [  582.390770]  __ip_finish_output+0x130/0x218 [  582.391218]  
>> ip_finish_output+0x38/0xd0 [  582.391633]  ip_output+0xb4/0x130 [  
>> 582.391984]  ip_local_out+0x58/0x68 [  582.392369]  
>> ip_send_skb+0x2c/0x88 [  582.392729]  ip_push_pending_frames+0x44/0x50 
>> [  582.393189]  raw_sendmsg+0x7a4/0x988 [  582.393569]  
>> inet_sendmsg+0x4c/0x78 [  582.393942]  sock_sendmsg+0x58/0x68 [  
>> 582.394311]  ____sys_sendmsg+0x284/0x2c0 [  582.394721]  
>> ___sys_sendmsg+0x90/0xd0 [  582.395113]  __sys_sendmsg+0x78/0xd0 [  
>> 582.395504]  __arm64_sys_sendmsg+0x2c/0x38 [  582.395942]  
>> el0_svc_common.constprop.2+0x70/0x128
>> [  582.396472]  do_el0_svc+0x34/0xa0
>> [  582.396834]  el0_sync_handler+0xec/0x128 [  582.397249]  
>> el0_sync+0x140/0x180 [  582.397611] Code: ab030005 91001442 9a030000 
>> 8b020882 (b8404423) [  582.398264] ---[ end trace 92adb54c8611f8c5 
>> ]--- [  582.398754] Kernel panic - not syncing: Fatal exception in 
>> interrupt [  582.399481] SMP: stopping secondary CPUs [  582.399923] 
>> Kernel Offset: 0xc0000 from 0xffff800010000000 [  582.400561] 
>> PHYS_OFFSET: 0x40000000 [  582.400939] CPU features: 0x040002,22a08238 
>> [  582.401380] Memory Limit: none [  582.401710] ---[ end Kernel panic 
>> - not syncing: Fatal exception in interrupt ]---
>>
>> This panic is because the ip header is corrupted. The ihl of the ip header is error,this cause ip_fast_csum access the illegal address.
>> 23 static inline __sum16 ip_fast_csum(const void *iph, unsigned int 
>> ihl)
>>  24 {
>>  25     __uint128_t tmp;
>>  26     u64 sum;
>>  27
>>  28     tmp = *(const __uint128_t *)iph;
>>  29     iph += 16;
>>  30     ihl -= 4;                                           -----here, if ihl is smaller than 5, the next will access the illegal address.
> 
> 
> Well, the bug is there then.
> 
> Code for other arches is different.
> 
> x86 for instance has a test for silly ihl
> 
> static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl) { unsigned int sum;
> 
> asm("  movl (%1), %0\n"
>     "  subl $4, %2\n"
>     "  jbe 2f\n"                          --- here
>     "  addl 4(%1), %0\n"
>     "  adcl 8(%1), %0\n"
>     "  adcl 12(%1), %0\n"
>     "1: adcl 16(%1), %0\n"
>     "  lea 4(%1), %1\n"
>     "  decl %2\n"
>     "  jne 1b\n"
>     "  adcl $0, %0\n"
>     "  movl %0, %2\n"
>     "  shrl $16, %0\n"
>     "  addw %w2, %w0\n"
>     "  adcl $0, %0\n"
>     "  notl %0\n"
>     "2:"
> /* Since the input registers which are loaded with iph and ihl
>    are modified, we must also specify them as outputs, or gcc
>    will assume they contain their original values. */
>     : "=r" (sum), "=r" (iph), "=r" (ihl)
>     : "1" (iph), "2" (ihl)
>     : "memory");
> return (__force __sum16)sum;
> }
> 
> 
>>  31     tmp += ((tmp >> 64) | (tmp << 64));
>>  32     sum = tmp >> 64;
>>  33     do {
>>  34         sum += *(const u32 *)iph;
>>  35         iph += 4;
>>  36     } while (--ihl);
>>  37
>>  38     sum += ((sum >> 32) | (sum << 32));
>>  39     return csum_fold((__force u32)(sum >> 32));
>>  40 }
>>
>> I think this panic may be a problem, thanks.
>>
>>
>>
>> -----邮件原件-----
>> 发件人: Cong Wang [mailto:xiyou.wangcong@gmail.com]
>> 发送时间: Wednesday, July 22, 2020 3:39
>> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
>> 抄送: David Miller <davem@davemloft.net>; Jakub Kicinski 
>> <kuba@kernel.org>; Mahesh Bandewar <maheshb@google.com>; Eric Dumazet 
>> <edumazet@google.com>; Linux Kernel Network Developers 
>> <netdev@vger.kernel.org>
>> 主题: Re: [PATCH] ipvlan: add the check of ip header checksum
>>
>> On Tue, Jul 21, 2020 at 6:17 AM guodeqing <geffrey.guo@huawei.com> wrote:
>>>
>>> The ip header checksum can be error in the following steps.
>>> $ ip netns add ns1
>>> $ ip link add gw link eth0 type ipvlan $ ip addr add 168.16.0.1/24 
>>> dev gw $ ip link set dev gw up $ ip link add ip1 link eth0 type 
>>> ipvlan $ ip link set ip1 netns ns1 $ ip netns exec ns1 ip link set 
>>> ip1 up $ ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 $ ip 
>>> netns exec ns1 tc qdisc add dev ip1 root netem corrupt 50% $ ip 
>>> netns exec ns1 ping
>>> 168.16.0.1
>>>
>>> The ip header of a packet maybe modified when it steps in 
>>> ipvlan_process_v4_outbound because of the netem, the corruptted 
>>> packets should be dropped.
>>
>> This does not make much sense, as you intentionally corrupt the header. More importantly, the check you add is too late, right?
>> ipvlan_xmit_mode_l3() already does the addr lookup with IP header,
>>
>> Thanks.
>>
>> -----邮件原件-----
>> 发件人: David Miller [mailto:davem@davemloft.net]
>> 发送时间: Thursday, July 23, 2020 4:04
>> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
>> 抄送: kuba@kernel.org; maheshb@google.com; edumazet@google.com; 
>> netdev@vger.kernel.org
>> 主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum
>>
>> From: guodeqing <geffrey.guo@huawei.com>
>> Date: Wed, 22 Jul 2020 17:18:19 +0800
>>
>>> The ip header checksum can be error in the following steps.
>>
>> You did not respond to my feedback from your previous submissions.
>>
>> Packets created inside of the kernel have correct checksums, and the ipvlan driver can depend upon this precondition.
>>
>> I am not applying this patch, sorry.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* 答复: 答复: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-24  5:13         ` Eric Dumazet
@ 2020-07-24  8:39           ` Guodeqing (A)
  0 siblings, 0 replies; 10+ messages in thread
From: Guodeqing (A) @ 2020-07-24  8:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, kuba, maheshb, netdev



> -----邮件原件-----
> 发件人: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> 发送时间: Friday, July 24, 2020 13:14
> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>; Eric Dumazet
> <edumazet@google.com>
> 抄送: David Miller <davem@davemloft.net>; kuba@kernel.org;
> maheshb@google.com; netdev@vger.kernel.org
> 主题: Re: 答复: [PATCH,v2] ipvlan: add the check of ip header checksum
> 
> Please do not top-post on netdev / lkml
> ( https://www.mediawiki.org/wiki/Mailing_list_etiquette )
> 

Thanks for your reminder.

> 
> 
> On 7/23/20 8:35 PM, Guodeqing (A) wrote:
> > The ihl check maybe not suitable in ip_fast_csum, the correct of the ihl value
> can be checked before calling the ip_fast_csum.
> >
> > The implementation of ip_fast_csum is different in different cpu architecture.
> the IP packet will do ip forward in the ipvlan l3/l3s mode and the corrupted ip
> packet
> >
> > should be discarded as soon as possible. Thanks.
> 
> Then the "as soon as possible" should be done in netem, right ?
> 

The corrupt function of netem is the emulation of random noise introducing an error in a random
position for a chosen percent of packets to test the network module.so the netem does not disgard
the packet.

> Alternatively fix arm64 ip_fast_csum() so that it matches the reference
> implementation (lib/checksum.c)
> 
> We do not want adding code in ipvlan that is essentially dead code for normal
> usages.
> 

Add the ip_fast_csum check in ipvlan_get_L3_hdr is reduncdant.I will modify the patch to fix the panic of doing 
the spectial test in arm64 VM.

> ip_fast_csum() is not free.
> 
> So, this is a NACK from my side, in case this was not clear enough.
> 
> 
> 
> >
> >
> > -----邮件原件-----
> > 发件人: Eric Dumazet [mailto:edumazet@google.com]
> > 发送时间: Thursday, July 23, 2020 10:15
> > 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> > 抄送: David Miller <davem@davemloft.net>; kuba@kernel.org;
> maheshb@google.com; netdev@vger.kernel.org
> > 主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum
> >
> > On Wed, Jul 22, 2020 at 6:59 PM Guodeqing (A) <geffrey.guo@huawei.com>
> wrote:
> >>
> >> I am sorry, the mail is not sent to you directly;
> >>
> >> If do the following test,this will cause a panic in a arm64 VM. this can be
> reproduced easily.
> >>
> >> Linux osc 5.8.0-rc6+ #3 SMP Thu Jul 23 01:40:47 UTC 2020 aarch64
> >>
> >> The programs included with the Debian GNU/Linux system are free
> >> software; the exact distribution terms for each program are described
> >> in the individual files in /usr/share/doc/*/copyright.
> >>
> >> Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
> >> permitted by applicable law.
> >> root@osc:~# ifconfig eth0 up
> >> root@osc:~# ip netns add ns1
> >> root@osc:~# ip link add gw link eth0 type ipvlan root@osc:~# ip addr
> >> add 168.16.0.1/24 dev gw root@osc:~# ip link set dev gw up root@osc:~#
> >> ip link add ip1 link eth0 type ipvlan root@osc:~# ip link set ip1
> >> netns ns1 root@osc:~# ip netns exec ns1 ip link set ip1 up root@osc:~#
> >> ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 root@osc:~# ip
> >> netns exec ns1 ip link set lo up root@osc:~# ip netns exec ns1 ip addr
> >> add 127.0.0.1/8 dev lo RTNETLINK answers: File exists root@osc:~# ip
> >> netns exec ns1 tc qdisc add dev ip1 root netem corrupt 100%
> >> root@osc:~# ip netns exec ns1 ping 168.16.0.1 PING 168.16.0.1
> >> (168.16.0.1) 56(84) bytes of data.
> >> From 168.16.0.1 icmp_seq=2 Destination Host Unreachable From
> >> 168.16.0.1 icmp_seq=12 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=30 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=48 ttl=64 time=0.052 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=65 ttl=64 time=0.060 ms From
> >> 168.16.0.1 icmp_seq=80 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=97 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=100 ttl=64 time=0.022 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=101 ttl=64 time=0.054 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=103 ttl=64 time=0.053 ms From
> >> 168.16.0.1 icmp_seq=102 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=127 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=132 ttl=64 time=0.057 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=135 ttl=64 time=0.051 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=141 ttl=64 time=0.051 ms From
> >> 168.16.0.1 icmp_seq=140 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=142 Destination Host Unreachable From 168.16.0.1
> icmp_seq=150
> >> Destination Host Unreachable From 168.16.0.1 icmp_seq=154 Destination
> >> Host Unreachable From 168.16.0.1 icmp_seq=164 Destination Host
> >> Unreachable From 168.16.0.1 icmp_seq=169 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=173 ttl=64 time=0.056 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=185 ttl=64 time=0.058 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=202 ttl=64 time=0.056 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=203 ttl=64 time=0.057 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=219 ttl=64 time=0.050 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=227 ttl=64 time=0.057 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=228 ttl=64 time=0.044 ms From
> >> 168.16.0.1 icmp_seq=237 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=239 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=243 ttl=64 time=0.053 ms From
> >> 168.16.0.1 icmp_seq=242 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=246 ttl=64 time=0.056 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=254 ttl=64 time=0.056 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=255 ttl=64 time=0.054 ms From
> >> 168.16.0.1 icmp_seq=263 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=269 Destination Host Unreachable From 168.16.0.1
> icmp_seq=273
> >> Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=279 ttl=64 time=0.057 ms From
> >> 168.16.0.1 icmp_seq=284 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=286 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=291 ttl=64 time=0.054 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=293 ttl=64 time=0.058 ms From
> >> 168.16.0.1 icmp_seq=294 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=298 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=309 ttl=64 time=0.056 ms From
> >> 168.16.0.1 icmp_seq=310 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=312 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=332 ttl=64 time=0.055 ms From
> >> 168.16.0.1 icmp_seq=334 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=340 ttl=64 time=0.055 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=344 ttl=64 time=0.050 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=355 ttl=64 time=0.057 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=358 ttl=64 time=0.050 ms From
> >> 168.16.0.1 icmp_seq=356 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=369 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=382 ttl=64 time=0.053 ms From
> >> 168.16.0.1 icmp_seq=381 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=396 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=400 ttl=64 time=0.058 ms From
> >> 168.16.0.1 icmp_seq=406 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=414 Destination Host Unreachable From 168.16.0.1
> icmp_seq=417
> >> Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=422 ttl=64 time=0.018 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=423 ttl=64 time=0.058 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=429 ttl=64 time=0.056 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=445 ttl=64 time=0.049 ms From
> >> 168.16.0.1 icmp_seq=444 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=453 Destination Host Unreachable From 168.16.0.1
> icmp_seq=455
> >> Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=456 ttl=64 time=0.024 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=469 ttl=64 time=0.058 ms From
> >> 168.16.0.1 icmp_seq=475 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=483 ttl=64 time=0.054 ms From
> >> 168.16.0.1 icmp_seq=488 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=504 ttl=64 time=0.056 ms
> >> 64 bytes from 168.16.0.1: icmp_seq=505 ttl=64 time=0.055 ms From
> >> 168.16.0.1 icmp_seq=510 Destination Host Unreachable From 168.16.0.1
> >> icmp_seq=511 Destination Host Unreachable
> >> 64 bytes from 168.16.0.1: icmp_seq=516 ttl=64 time=0.055 ms From
> >> 168.16.0.1 icmp_seq=519 Destination Host Unreachable [  582.368938]
> >> Unable to handle kernel paging request at virtual address
> >> ffff0000f85f0000 [  582.369732] Mem abort info:
> >> [  582.369987]   ESR = 0x96000007
> >> [  582.370266]   EC = 0x25: DABT (current EL), IL = 32 bits
> >> [  582.370833]   SET = 0, FnV = 0
> >> [  582.371113]   EA = 0, S1PTW = 0
> >> [  582.371391] Data abort info:
> >> [  582.371671]   ISV = 0, ISS = 0x00000007
> >> [  582.372017]   CM = 0, WnR = 0
> >> [  582.372299] swapper pgtable: 4k pages, 48-bit VAs,
> >> pgdp=000000012dab7000 [  582.372896] [ffff0000f85f0000]
> >> pgd=000000013fff8003, p4d=000000013fff8003, pud=000000013f9f4003,
> >> pmd=000000013f838003, pte=0000000000000000 [  582.374033] Internal
> error: Oops: 96000007 [#1] SMP [  582.374468] Modules linked in:
> >> [  582.374795] CPU: 1 PID: 525 Comm: ping Not tainted 5.8.0-rc6+ #3 [
> >> 582.375468] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0
> >> 02/06/2015 [  582.376215] pstate: 20400005 (nzCv daif +PAN -UAO
> >> BTYPE=--) [  582.376805] pc : __ip_local_out+0x84/0x188 [  582.377234]
> >> lr : ip_local_out+0x34/0x68 [  582.377635] sp : ffff800013263440 [
> >> 582.377986] x29: ffff800013263440 x28: 0000000000000001
> [  582.378536]
> >> x27: ffff8000111d2018 x26: ffff8000114cba80 [  582.379093] x25:
> >> ffff0000ec4e7400 x24: 0000000000000000 [  582.379653] x23:
> >> 0000000000000062 x22: ffff8000114c9000 [  582.380221] x21:
> >> ffff0000d97ac600 x20: ffff0000ec519000 [  582.380778] x19:
> >> ffff8000115b5bc0 x18: 0000000000000000 [  582.381324] x17:
> >> 0000000000000000 x16: 0000000000000000 [  582.381876] x15:
> >> 0000000000000000 x14: 0000000000000000 [  582.382431] x13:
> >> 0000000000000000 x12: 0000000000000001 [  582.382986] x11:
> >> ffff800010d21838 x10: 0000000000000001 [  582.383567] x9 :
> >> 0000000000000001 x8 : 0000000000000000 [  582.384136] x7 :
> >> 0000000000000000 x6 : ffff0000ec4e5e00 [  582.384693] x5 :
> >> 024079ca54000184 x4 : ffff0000ec4e5e10 [  582.385246] x3 :
> >> 0000000000000000 x2 : ffff0004ec4e5e20 [  582.385808] x1 :
> >> ffff0000f85f0000 x0 : 031d079626a9c7ae [  582.386365] Call trace:
> >> [  582.386629]  __ip_local_out+0x84/0x188 [  582.387030]
> >> ip_local_out+0x34/0x68 [  582.387400]  ipvlan_queue_xmit+0x548/0x5c0
> [
> >> 582.387845]  ipvlan_start_xmit+0x2c/0x90 [  582.388283]
> >> dev_hard_start_xmit+0xb4/0x260 [  582.388732]
> >> sch_direct_xmit+0x1b4/0x550 [  582.389145]  __qdisc_run+0x140/0x648
> [
> >> 582.389524]  __dev_queue_xmit+0x6a4/0x8b8 [  582.389948]
> >> dev_queue_xmit+0x24/0x30 [  582.390339]
> ip_finish_output2+0x324/0x580
> >> [  582.390770]  __ip_finish_output+0x130/0x218 [  582.391218]
> >> ip_finish_output+0x38/0xd0 [  582.391633]  ip_output+0xb4/0x130 [
> >> 582.391984]  ip_local_out+0x58/0x68 [  582.392369]
> >> ip_send_skb+0x2c/0x88 [  582.392729]
> ip_push_pending_frames+0x44/0x50
> >> [  582.393189]  raw_sendmsg+0x7a4/0x988 [  582.393569]
> >> inet_sendmsg+0x4c/0x78 [  582.393942]  sock_sendmsg+0x58/0x68 [
> >> 582.394311]  ____sys_sendmsg+0x284/0x2c0 [  582.394721]
> >> ___sys_sendmsg+0x90/0xd0 [  582.395113]  __sys_sendmsg+0x78/0xd0 [
> >> 582.395504]  __arm64_sys_sendmsg+0x2c/0x38 [  582.395942]
> >> el0_svc_common.constprop.2+0x70/0x128
> >> [  582.396472]  do_el0_svc+0x34/0xa0
> >> [  582.396834]  el0_sync_handler+0xec/0x128 [  582.397249]
> >> el0_sync+0x140/0x180 [  582.397611] Code: ab030005 91001442
> 9a030000
> >> 8b020882 (b8404423) [  582.398264] ---[ end trace 92adb54c8611f8c5
> >> ]--- [  582.398754] Kernel panic - not syncing: Fatal exception in
> >> interrupt [  582.399481] SMP: stopping secondary CPUs [  582.399923]
> >> Kernel Offset: 0xc0000 from 0xffff800010000000 [  582.400561]
> >> PHYS_OFFSET: 0x40000000 [  582.400939] CPU features:
> 0x040002,22a08238
> >> [  582.401380] Memory Limit: none [  582.401710] ---[ end Kernel panic
> >> - not syncing: Fatal exception in interrupt ]---
> >>
> >> This panic is because the ip header is corrupted. The ihl of the ip header is
> error,this cause ip_fast_csum access the illegal address.
> >> 23 static inline __sum16 ip_fast_csum(const void *iph, unsigned int
> >> ihl)
> >>  24 {
> >>  25     __uint128_t tmp;
> >>  26     u64 sum;
> >>  27
> >>  28     tmp = *(const __uint128_t *)iph;
> >>  29     iph += 16;
> >>  30     ihl -= 4;
> -----here, if ihl is smaller than 5, the next will access the illegal address.
> >
> >
> > Well, the bug is there then.
> >
> > Code for other arches is different.
> >
> > x86 for instance has a test for silly ihl
> >
> > static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
> { unsigned int sum;
> >
> > asm("  movl (%1), %0\n"
> >     "  subl $4, %2\n"
> >     "  jbe 2f\n"                          --- here
> >     "  addl 4(%1), %0\n"
> >     "  adcl 8(%1), %0\n"
> >     "  adcl 12(%1), %0\n"
> >     "1: adcl 16(%1), %0\n"
> >     "  lea 4(%1), %1\n"
> >     "  decl %2\n"
> >     "  jne 1b\n"
> >     "  adcl $0, %0\n"
> >     "  movl %0, %2\n"
> >     "  shrl $16, %0\n"
> >     "  addw %w2, %w0\n"
> >     "  adcl $0, %0\n"
> >     "  notl %0\n"
> >     "2:"
> > /* Since the input registers which are loaded with iph and ihl
> >    are modified, we must also specify them as outputs, or gcc
> >    will assume they contain their original values. */
> >     : "=r" (sum), "=r" (iph), "=r" (ihl)
> >     : "1" (iph), "2" (ihl)
> >     : "memory");
> > return (__force __sum16)sum;
> > }
> >
> >
> >>  31     tmp += ((tmp >> 64) | (tmp << 64));
> >>  32     sum = tmp >> 64;
> >>  33     do {
> >>  34         sum += *(const u32 *)iph;
> >>  35         iph += 4;
> >>  36     } while (--ihl);
> >>  37
> >>  38     sum += ((sum >> 32) | (sum << 32));
> >>  39     return csum_fold((__force u32)(sum >> 32));
> >>  40 }
> >>
> >> I think this panic may be a problem, thanks.
> >>
> >>
> >>
> >> -----邮件原件-----
> >> 发件人: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> >> 发送时间: Wednesday, July 22, 2020 3:39
> >> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> >> 抄送: David Miller <davem@davemloft.net>; Jakub Kicinski
> >> <kuba@kernel.org>; Mahesh Bandewar <maheshb@google.com>; Eric
> Dumazet
> >> <edumazet@google.com>; Linux Kernel Network Developers
> >> <netdev@vger.kernel.org>
> >> 主题: Re: [PATCH] ipvlan: add the check of ip header checksum
> >>
> >> On Tue, Jul 21, 2020 at 6:17 AM guodeqing <geffrey.guo@huawei.com>
> wrote:
> >>>
> >>> The ip header checksum can be error in the following steps.
> >>> $ ip netns add ns1
> >>> $ ip link add gw link eth0 type ipvlan $ ip addr add 168.16.0.1/24
> >>> dev gw $ ip link set dev gw up $ ip link add ip1 link eth0 type
> >>> ipvlan $ ip link set ip1 netns ns1 $ ip netns exec ns1 ip link set
> >>> ip1 up $ ip netns exec ns1 ip addr add 168.16.0.2/24 dev ip1 $ ip
> >>> netns exec ns1 tc qdisc add dev ip1 root netem corrupt 50% $ ip
> >>> netns exec ns1 ping
> >>> 168.16.0.1
> >>>
> >>> The ip header of a packet maybe modified when it steps in
> >>> ipvlan_process_v4_outbound because of the netem, the corruptted
> >>> packets should be dropped.
> >>
> >> This does not make much sense, as you intentionally corrupt the header.
> More importantly, the check you add is too late, right?
> >> ipvlan_xmit_mode_l3() already does the addr lookup with IP header,
> >>
> >> Thanks.
> >>
> >> -----邮件原件-----
> >> 发件人: David Miller [mailto:davem@davemloft.net]
> >> 发送时间: Thursday, July 23, 2020 4:04
> >> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> >> 抄送: kuba@kernel.org; maheshb@google.com; edumazet@google.com;
> >> netdev@vger.kernel.org
> >> 主题: Re: [PATCH,v2] ipvlan: add the check of ip header checksum
> >>
> >> From: guodeqing <geffrey.guo@huawei.com>
> >> Date: Wed, 22 Jul 2020 17:18:19 +0800
> >>
> >>> The ip header checksum can be error in the following steps.
> >>
> >> You did not respond to my feedback from your previous submissions.
> >>
> >> Packets created inside of the kernel have correct checksums, and the ipvlan
> driver can depend upon this precondition.
> >>
> >> I am not applying this patch, sorry.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: 答复: [PATCH,v2] ipvlan: add the check of ip header checksum
  2020-07-24  3:35       ` 答复: " Guodeqing (A)
  2020-07-24  5:13         ` Eric Dumazet
@ 2020-07-24 23:44         ` David Miller
  1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2020-07-24 23:44 UTC (permalink / raw)
  To: geffrey.guo; +Cc: edumazet, kuba, maheshb, netdev

From: "Guodeqing (A)" <geffrey.guo@huawei.com>
Date: Fri, 24 Jul 2020 03:35:02 +0000

> The ihl check maybe not suitable in ip_fast_csum, the correct of the
> ihl value can be checked before calling the ip_fast_csum.

ip_fast_csum() must be able to handle any value that could fit in the
ihl field of the ip protocol header.  That's not only the most correct
logic, but also the most robust.

> The implementation of ip_fast_csum is different in different cpu
> architecture. the IP packet will do ip forward in the ipvlan l3/l3s
> mode and the corrupted ip packet

As Eric explained, several times, ip_fast_csum on arm64 has a bug and that
is where the fix belongs.

Please fix this bug in the proper place.

Thank you.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-24 23:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-22  9:18 [PATCH,v2] ipvlan: add the check of ip header checksum guodeqing
2020-07-22 16:29 ` Eric Dumazet
2020-07-23  2:02   ` 答复: " Guodeqing (A)
2020-07-22 20:04 ` David Miller
2020-07-23  1:59   ` 答复: " Guodeqing (A)
2020-07-23  2:15     ` Eric Dumazet
2020-07-24  3:35       ` 答复: " Guodeqing (A)
2020-07-24  5:13         ` Eric Dumazet
2020-07-24  8:39           ` 答复: " Guodeqing (A)
2020-07-24 23:44         ` David Miller

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).