netdev - Re: [PATCH net] ipv4: Avoid crashing in ip

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: <87oalb2w8l.fsf@x220.int.ebiederm.org>
Date:	Sat, 23 May 2015 09:40:10 -0500
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Julian Anastasov <ja@....bg>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	"Vittorio G \(VittGam\)" <linuxbugs@...tgam.net>,
	security@...nel.org, Greg KH <greg@...ah.com>, luto@...capital.net,
	Willy Tarreau <w@....eu>
Subject: Re: [PATCH net] ipv4: Avoid crashing in ip_error

Julian Anastasov <ja@....bg> writes:

> 	Hello,
>
> On Fri, 22 May 2015, Eric W. Biederman wrote:
>
>> ip_error does not check if in_dev is NULL before dereferencing it.
>> 
>> IThe following sequence of calls is possible:
>> CPU A                          CPU B
>> ip_rcv_finish
>>     ip_route_input_noref()
>>         ip_route_input_slow()
>>                                inetdev_destroy()
>>     dst_input()
>> 
>> With the result that a network device can be destroyed while processing
>> an input packet.
>
> 	How can that happen in practice? May be network
> driver is missing a napi_disable() call in ndo_stop()?
> napi_disable is supposed to wait backlog before NETDEV_DOWN
> and NETDEV_UNREGISTER (where inetdev_destroy is called) handlers
> are notified. Any link to original crash report, info for used
> driver, etc?
>
> 	After commit 8030f54499925d ("[IPV4] devinet: Register inetdev 
> earlier.") dev->ip_ptr is always present while packets are
> processed, only in_dev->ifa_list can be NULL.

Interesting I see the dev_close in unregister_netdevice that should have
prevented this scenario.

The reproducer was apparently:
ip netns add ns0
ip netns add ns1
ip link add test0 type veth peer name test1
ip link set test0 netns ns0
ip link set test1 netns ns1
ip netns exec ns0 sysctl net.ipv4.ip_forward=1
ip netns exec ns1 sysctl net.ipv4.ip_forward=1
ip netns exec ns0 sysctl net.ipv4.conf.all.accept_local=1
ip netns exec ns1 sysctl net.ipv4.conf.all.accept_local=1
ip netns exec ns0 sysctl net.ipv4.conf.test0.accept_local=1
ip netns exec ns1 sysctl net.ipv4.conf.test1.accept_local=1
ip netns exec ns0 sysctl net.ipv4.conf.all.send_redirects=0
ip netns exec ns1 sysctl net.ipv4.conf.all.send_redirects=0
ip netns exec ns0 sysctl net.ipv4.conf.test0.send_redirects=0
ip netns exec ns1 sysctl net.ipv4.conf.test1.send_redirects=0
ip netns exec ns0 ip addr add 10.10.10.1/24 dev test0
ip netns exec ns1 ip addr add 10.10.10.2/24 dev test1
ip netns exec ns0 ip link set up dev test0
ip netns exec ns1 ip link set up dev test1
ip netns exec ns0 ip route add 10.20.30.40 via 10.10.10.2 dev test0
ip netns exec ns1 ip route add 10.20.30.40 via 10.10.10.1 dev test1
ip netns exec ns0 iptables -t mangle -A PREROUTING -i test0 -j TTL --ttl-inc 1
ip netns exec ns1 iptables -t mangle -A PREROUTING -i test1 -j TTL --ttl-inc 1
ip netns exec ns0 ping -s 1400 -f -w 5 10.20.30.40
ip netns delete ns0
ip netns delete ns1

With a NULL pointer dereference in ip_error at offset 0x108 which
appeared to correspond with the line:

	if (!IN_DEV_FORWARD(in_dev)) {

As did the debug information.

Plus there is not much other than in_dev that could be NULL in that
function.

Perhaps it is possible to transmit into a veth pair even when the
carrier is down?  Which could be a bug in the veth driver.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html