| Message ID | 1595336962-98677-1-git-send-email-geffrey.guo@huawei.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipvlan: add the check of ip header checksum | expand |
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.
From: guodeqing <geffrey.guo@huawei.com> Date: Tue, 21 Jul 2020 21:09:22 +0800 > The ip header checksum can be error in the following steps. ... > $ ip netns exec ns1 tc qdisc add dev ip1 root netem corrupt 50% This is not valid. The kernel internally already validated the ipv4 header checksum before forwarding or sending it on egress to the ipvlan device. The driver can legitimately depend upon this validation. Besides, as Cong pointed out, so much other code in the ipvlan driver has read various ipv4 header members such as the addresses necessary to perform lookups.
This check of ip head checksum should be add in ipvlan_get_L3_hdr，if the ipvlan_get_L3_hdr return value is null, the packet should be dropped. Because the IP packet will do ip forward in the ipvlan l3/l3s mode and the corrupted ip packet should not go ahead, the other type packet will be dropped in ipvlan_process_outbound latter. If the corrupted ip packet go ahead,this will cause a panic in a arm64 VM. Linux 5.8.0-rc6+ #1 SMP Wed Jul 22 07:57:11 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. # ifconfig eth0 up # 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 ip link set lo up # ip netns exec ns1 ip addr add 127.0.0.1/8 dev lo RTNETLINK answers: File exists # ip netns exec ns1 tc qdisc add dev ip1 root netem corrupt 100% # ip netns exec ns1 ping 168.16.0.1 PING 168.16.0.1 (168.16.0.1) 56(84) bytes of data. 64 bytes from 168.16.0.1: icmp_seq=14 ttl=64 time=0.063 ms 64 bytes from 168.16.0.1: icmp_seq=18 ttl=64 time=0.060 ms From 168.16.0.1 icmp_seq=22 Destination Host Unreachable From 168.16.0.1 icmp_seq=26 Destination Host Unreachable From 168.16.0.1 icmp_seq=50 Destination Host Unreachable 64 bytes from 168.16.0.1: icmp_seq=62 ttl=64 time=0.051 ms 64 bytes from 168.16.0.1: icmp_seq=64 ttl=64 time=0.059 ms 64 bytes from 168.16.0.1: icmp_seq=70 ttl=64 time=0.056 ms 64 bytes from 168.16.0.1: icmp_seq=100 ttl=64 time=0.058 ms 64 bytes from 168.16.0.1: icmp_seq=114 ttl=64 time=0.059 ms 64 bytes from 168.16.0.1: icmp_seq=115 ttl=64 time=0.052 ms 64 bytes from 168.16.0.1: icmp_seq=117 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=134 ttl=64 time=0.059 ms From 168.16.0.1 icmp_seq=164 Destination Host Unreachable From 168.16.0.1 icmp_seq=183 Destination Host Unreachable 64 bytes from 168.16.0.1: icmp_seq=253 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=256 ttl=64 time=0.060 ms 64 bytes from 168.16.0.1: icmp_seq=258 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=262 ttl=64 time=0.046 ms 64 bytes from 168.16.0.1: icmp_seq=274 ttl=64 time=0.062 ms 64 bytes from 168.16.0.1: icmp_seq=278 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=293 ttl=64 time=0.062 ms From 168.16.0.1 icmp_seq=292 Destination Host Unreachable 64 bytes from 168.16.0.1: icmp_seq=309 ttl=64 time=0.065 ms 64 bytes from 168.16.0.1: icmp_seq=322 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=329 ttl=64 time=0.041 ms 64 bytes from 168.16.0.1: icmp_seq=377 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=380 ttl=64 time=0.062 ms 64 bytes from 168.16.0.1: icmp_seq=385 ttl=64 time=0.065 ms From 168.16.0.1 icmp_seq=384 Destination Host Unreachable 64 bytes from 168.16.0.1: icmp_seq=389 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=392 ttl=64 time=0.060 ms 64 bytes from 168.16.0.1: icmp_seq=398 ttl=64 time=0.061 ms 64 bytes from 168.16.0.1: icmp_seq=399 ttl=64 time=0.062 ms 64 bytes from 168.16.0.1: icmp_seq=402 ttl=64 time=0.056 ms 64 bytes from 168.16.0.1: icmp_seq=435 ttl=64 time=0.053 ms 64 bytes from 168.16.0.1: icmp_seq=443 ttl=64 time=0.054 ms 64 bytes from 168.16.0.1: icmp_seq=448 ttl=64 time=0.059 ms 64 bytes from 168.16.0.1: icmp_seq=450 ttl=64 time=0.059 ms 64 bytes from 168.16.0.1: icmp_seq=456 ttl=64 time=0.060 ms [ 506.647291] Unable to handle kernel paging request at virtual address ffff0000f85f0000 [ 506.648121] Mem abort info: [ 506.648481] ESR = 0x96000007 [ 506.648777] EC = 0x25: DABT (current EL), IL = 32 bits [ 506.649270] SET = 0, FnV = 0 [ 506.649698] EA = 0, S1PTW = 0 [ 506.650103] Data abort info: [ 506.650407] ISV = 0, ISS = 0x00000007 [ 506.651025] CM = 0, WnR = 0 [ 506.651363] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000012dab7000 [ 506.652252] [ffff0000f85f0000] pgd=000000013fff8003, p4d=000000013fff8003, pud=000000013f9f4003, pmd=000000013f838003, pte=0000000000000000 [ 506.653729] Internal error: Oops: 96000007 [#1] SMP [ 506.654316] Modules linked in: [ 506.654748] CPU: 0 PID: 526 Comm: ping Not tainted 5.8.0-rc6+ #1 [ 506.655594] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015 [ 506.656604] pstate: 20400005 (nzCv daif +PAN -UAO BTYPE=--) [ 506.657432] pc : __ip_local_out+0x84/0x188 [ 506.658061] lr : ip_local_out+0x34/0x68 [ 506.658602] sp : ffff80001324b440 [ 506.659116] x29: ffff80001324b440 x28: 0000000000000001 [ 506.659913] x27: ffff8000111d2018 x26: ffff8000114cba80 [ 506.660703] x25: ffff0000ec419400 x24: 0000000000000000 [ 506.661578] x23: 0000000000000062 x22: ffff8000114c9000 [ 506.662401] x21: ffff0000fe4c3d00 x20: ffff0000ec4bd800 [ 506.663180] x19: ffff8000115b5bc0 x18: 0000000000000000 [ 506.663999] x17: 0000000000000000 x16: 0000000000000000 [ 506.664749] x15: 0000000000000000 x14: 0000000000000000 [ 506.665544] x13: 0000000000000000 x12: 0000000000000001 [ 506.666333] x11: ffff800010d21838 x10: 0000000000000001 [ 506.667145] x9 : 0000000000000001 x8 : 0000000000000000 [ 506.667921] x7 : 0000000000000000 x6 : ffff0000ec531200 [ 506.668682] x5 : 0240334854000184 x4 : ffff0000ec531210 [ 506.669529] x3 : 0000000000000000 x2 : ffff0004ec531220 [ 506.670311] x1 : ffff0000f85f0000 x0 : 031dd0d820c74d91 [ 506.671116] Call trace: [ 506.671516] __ip_local_out+0x84/0x188 [ 506.672077] ip_local_out+0x34/0x68 [ 506.672618] ipvlan_queue_xmit+0x548/0x5c0 [ 506.673222] ipvlan_start_xmit+0x2c/0x90 [ 506.673863] dev_hard_start_xmit+0xb4/0x260 [ 506.674526] sch_direct_xmit+0x1b4/0x550 [ 506.675126] __qdisc_run+0x140/0x648 [ 506.675662] __dev_queue_xmit+0x6a4/0x8b8 [ 506.676259] dev_queue_xmit+0x24/0x30 [ 506.676813] ip_finish_output2+0x324/0x580 [ 506.677430] __ip_finish_output+0x130/0x218 [ 506.678084] ip_finish_output+0x38/0xd0 [ 506.678647] ip_output+0xb4/0x130 [ 506.679123] ip_local_out+0x58/0x68 [ 506.679629] ip_send_skb+0x2c/0x88 [ 506.680140] ip_push_pending_frames+0x44/0x50 [ 506.680798] raw_sendmsg+0x7a4/0x988 [ 506.681343] inet_sendmsg+0x4c/0x78 [ 506.681916] sock_sendmsg+0x58/0x68 [ 506.682440] ____sys_sendmsg+0x284/0x2c0 [ 506.683060] ___sys_sendmsg+0x90/0xd0 [ 506.683605] __sys_sendmsg+0x78/0xd0 [ 506.684150] __arm64_sys_sendmsg+0x2c/0x38 [ 506.684787] el0_svc_common.constprop.2+0x70/0x128 [ 506.685525] do_el0_svc+0x34/0xa0 [ 506.686018] el0_sync_handler+0xec/0x128 [ 506.686611] el0_sync+0x140/0x180 [ 506.687123] Code: ab030005 91001442 9a030000 8b020882 (b8404423) [ 506.688020] ---[ end trace 016adaee3c782b08 ]--- [ 506.688694] Kernel panic - not syncing: Fatal exception in interrupt [ 506.689669] SMP: stopping secondary CPUs [ 506.690283] Kernel Offset: 0xc0000 from 0xffff800010000000 [ 506.691043] PHYS_OFFSET: 0x40000000 [ 506.691558] CPU features: 0x040002,22a08238 [ 506.692170] Memory Limit: none [ 506.692642] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]--- This panic is because the ip header is corrupted. I think this panic may be a problem, I will modify the patch to fix this panic,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.
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c index 8801d09..22cef89 100644 --- a/drivers/net/ipvlan/ipvlan_core.c +++ b/drivers/net/ipvlan/ipvlan_core.c @@ -428,6 +428,12 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb) .saddr = ip4h->saddr, }; + if (ip4h->ihl < 5) + goto err; + + if (unlikely(ip_fast_csum((u8 *)ip4h, ip4h->ihl))) + goto err; + rt = ip_route_output_flow(net, &fl4, NULL); if (IS_ERR(rt)) goto err;
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. Here I add the check of ip header to drop the corruptted packets and to avoid a problem in some cases. Signed-off-by: guodeqing <geffrey.guo@huawei.com> --- drivers/net/ipvlan/ipvlan_core.c | 6 ++++++ 1 file changed, 6 insertions(+)