netdev - Re: [RFC PATCH net] vrf: fix incorrect dereferencing of skb->cb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4383fcc3-f7de-8eb3-6746-2f271578a9e0@kernel.org>
Date:   Wed, 16 Feb 2022 20:50:06 -0700
From:   David Ahern <dsahern@...nel.org>
To:     "D. Wythe" <alibuda@...ux.alibaba.com>
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org
Subject: Re: [RFC PATCH net] vrf: fix incorrect dereferencing of skb->cb

On 2/14/22 6:10 AM, D. Wythe wrote:
> From: "D. Wythe" <alibuda@...ux.alibaba.com>
> 
> There is a failed test case in vrf scenario, it can be reproduced
> with script:
> 
> ./vrf_route_leaking.sh -t ipv6_ping_frag
> 
> Which output is similar to following:
> 
> TEST: Basic IPv6 connectivity			[ OK ]
> TEST: Ping received ICMP Packet too big		[FAIL]
> 
> The direct cause of this issue is because the Packet too big in ICMPv6
> is sent by the message whose source address is IPv6 loopback address and
> is discarded dues to its input interface is not the local loopback device.
> But the root cause is there's a bug that affects the correct source
> address selection.

That is the correct problem, but your solution is not.

> 
> In the above case the calling stack is as follows:
> 
>     icmp6_send+1
>     ip6_fragment+543
>     ip6_output+98
>     vrf_ip6_local_out+78
>     vrf_xmit+424
>     dev_hard_start_xmit+221
>     __dev_queue_xmit+2792
>     ip6_finish_output2+705
>     ip6_output+98
>     ip6_forward+1464
>     ipv6_rcv+67
> 
> To be more specific:
> 
> ipv6_rcv()
> 	init skb->cb as struct inet6_skb_parm
> ...
> __dev_queue_xmit()
> 	qdisc_pkt_len_init()
> 	init skb->cb as struct qdisc_skb_cb
> ...
> vrf_xmit
> 	fill skb->cb to zero.
> ...
> ip6_fragment()
> icmp6_send()
> 	treats skb->cb as struct inet6_skb_parm.
> 
> icmp6_send() will try to use original input interface in IP6CB for
> selecting a more meaningful source address, we should keep the old IP6CB
> rather than overwrite it.

The packet is recirculated through the VRF device so the cb data should
be overwritten. The VRF driver does another route lookup within the VRF.
Address selection should only consider addresses in the domain.
Something is off in that logic for VRF route leaking. I looked into a
bit when Michael posted the tests, but never came back to it.