答复: [PATCH,v2] arm64: fix the illegal address access in some cases

["Guodeqing (A)" <geffrey.guo@huawei.com>]

> -----邮件原件-----
> 发件人: Catalin Marinas [mailto:catalin.marinas@arm.com]
> 发送时间: Monday, July 27, 2020 19:47
> 收件人: Guodeqing (A) <geffrey.guo@huawei.com>
> 抄送: robin.murphy@arm.com; luke.starrett@broadcom.com; will@kernel.org;
> linux-arm-kernel@lists.infradead.org
> 主题: Re: [PATCH,v2] arm64: fix the illegal address access in some cases
> 
> On Sat, Jul 25, 2020 at 10:08:06AM +0800, guodeqing wrote:
> > The ihl value of ip header is smaller than 5 in some cases, if the ihl
> > value is smaller than 5, then the next code will access the illegal
> > address, and the system will panic. ip_fast_csum() must be able to
> > handle any value that could fit in the ihl field of the ip protocol header.
> >
> > Here I add the check of the ihl value to solve this problem.
> >
> > Fixes: 0e455d8e80aa (arm64: Implement optimised IP checksum helpers)
> > Signed-off-by: guodeqing <geffrey.guo@huawei.com>
> > ---
> >  arch/arm64/include/asm/checksum.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/checksum.h
> > b/arch/arm64/include/asm/checksum.h
> > index b6f7bc6..5a7d9ac 100644
> > --- a/arch/arm64/include/asm/checksum.h
> > +++ b/arch/arm64/include/asm/checksum.h
> > @@ -25,6 +25,9 @@ static inline __sum16 ip_fast_csum(const void *iph,
> unsigned int ihl)
> >  	__uint128_t tmp;
> >  	u64 sum;
> >
> > +	if (unlikely(ihl < 5))
> > +		return 1;
> > +
> >  	tmp = *(const __uint128_t *)iph;
> >  	iph += 16;
> >  	ihl -= 4;
> 
> IHL in IPv4 should be at least 5. Do you have a stack trace to show how it got
> here? Maybe the caller should ensure that the correct size is passed.
> 

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

~# 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 
~# 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

[  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 ]---

Do the same test in the x86_64 VM will not cause a panic. this is because ip_fast_csum in the x86_64 architecture has the check
of the ihl value.

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;
}

Thanks.



> Thanks.
> 
> --
> Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel