[PATCH,v3] arm64: fix the illegal address access in some cases

Robin Murphy robin.murphy at arm.com
Thu Jul 30 08:01:44 EDT 2020


On 2020-07-30 12:24, guodeqing wrote:
> If do the following test in the arm64 VM, a panic will be produced.
> $ 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
> 
>   | Unable to handle kernel paging request at virtual address
>   | Internal error: Oops: 96000007 [#1] SMP
>   | CPU: 1 PID: 525 Comm: ping Not tainted 5.8.0-rc6+ #3
>   | pstate: 20400005 (nzCv daif +PAN -UAO BTYPE=--)
>   | pc : __ip_local_out+0x84/0x188
>   | lr : ip_local_out+0x34/0x68 sp : ffff800013263440
>   | x29: ffff800013263440 x28: 0000000000000001
>   | x27: ffff8000111d2018 x26: ffff8000114cba80
>   | x25: ffff0000ec4e7400 x24: 0000000000000000
>   | x23: 0000000000000062 x22: ffff8000114c9000
>   | x21: ffff0000d97ac600 x20: ffff0000ec519000
>   | x19: ffff8000115b5bc0 x18: 0000000000000000
>   | x17: 0000000000000000 x16: 0000000000000000
>   | x15: 0000000000000000 x14: 0000000000000000
>   | x13: 0000000000000000 x12: 0000000000000001
>   | x11: ffff800010d21838 x10: 0000000000000001
>   | x9 : 0000000000000001 x8 : 0000000000000000
>   | x7 : 0000000000000000 x6 : ffff0000ec4e5e00
>   | x5 : 024079ca54000184 x4 : ffff0000ec4e5e10
>   | x3 : 0000000000000000 x2 : ffff0004ec4e5e20
>   | x1 : ffff0000f85f0000 x0 : 031d079626a9c7ae
>   | Call trace:
>   |  __ip_local_out+0x84/0x188
>   |  ip_local_out+0x34/0x68
>   |  ipvlan_queue_xmit+0x548/0x5c0
>   |  ipvlan_start_xmit+0x2c/0x90
>   |  dev_hard_start_xmit+0xb4/0x260
>   |  sch_direct_xmit+0x1b4/0x550
>   |  __qdisc_run+0x140/0x648
>   |  __dev_queue_xmit+0x6a4/0x8b8
>   |  dev_queue_xmit+0x24/0x30
>   |  ip_finish_output2+0x324/0x580
>   |  __ip_finish_output+0x130/0x218
>   |  ip_finish_output+0x38/0xd0
>   |  ip_output+0xb4/0x130
> 
> Here I add the check of the ihl value to fix the problem.

Can you confirm that the inline patch I sent also fixes it? I'd prefer 
that as it has no impact for correct cases.

It would also have been useful to explain *why* this case causes a 
crash, since it's not all that obvious - having eventually figured it 
out by inspection, and knowing that compilers like to transform index 
comparisons into pointer comparisons, x4 and x2 in the register dump 
then start to stand out as the smoking gun, but it took me a while to 
get there, since the nature of your fix implies that accessing within 
the assumed bounds of iph itself was a problem, when in fact it isn't.

> Fixes: 0e455d8e80aa (arm64: Implement optimised IP checksum helpers)
> Signed-off-by: guodeqing <geffrey.guo at 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..702ac89 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 (WARN_ON_ONCE(ihl < 5))

My reading of the netdev discussion is that if we're expected to be 
robust to this due to possible Rx packet corruption, then WARN probably 
isn't appropriate.

Robin.

> +		ihl = 5;
> +
>   	tmp = *(const __uint128_t *)iph;
>   	iph += 16;
>   	ihl -= 4;
> 



More information about the linux-arm-kernel mailing list