netdev - [net PATCH] hsr: fix a NULL pointer dereference in hsr_dev

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-next>] [day] [month] [year] [list]
Message-Id: <20191130142400.3930-1-ap420073@gmail.com>
Date:   Sat, 30 Nov 2019 14:24:00 +0000
From:   Taehee Yoo <ap420073@...il.com>
To:     davem@...emloft.net, xiyou.wangcong@...il.com,
        netdev@...r.kernel.org
Cc:     ap420073@...il.com, treeze.taeung@...il.com
Subject: [net PATCH] hsr: fix a NULL pointer dereference in hsr_dev_xmit()

hsr_dev_xmit() calls hsr_port_get_hsr() to find master node and that would
return NULL if master node is not existing in the list.
But hsr_dev_xmit() doesn't check return pointer so a NULL dereference
could occur.

In the TX datapath, there is no rcu_read_lock() so this patch adds missing
rcu_read_lock() in the hsr_dev_xmit() too.

Test commands:
    ip netns add nst
    ip link add v0 type veth peer name v1
    ip link add v2 type veth peer name v3
    ip link set v1 netns nst
    ip link set v3 netns nst
    ip link add hsr0 type hsr slave1 v0 slave2 v2
    ip a a 192.168.100.1/24 dev hsr0
    ip link set v0 up
    ip link set v2 up
    ip link set hsr0 up
    ip netns exec nst ip link add hsr1 type hsr slave1 v1 slave2 v3
    ip netns exec nst ip a a 192.168.100.2/24 dev hsr1
    ip netns exec nst ip link set v1 up
    ip netns exec nst ip link set v3 up
    ip netns exec nst ip link set hsr1 up
    hping3 192.168.100.2 -2 --flood &
    modprobe -rv hsr

Splat looks like:
[  390.879740][ T1362] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[  390.880789][ T1362] CPU: 3 PID: 1362 Comm: hping3 Not tainted 5.4.0+ #183
[  390.881679][ T1362] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  390.882804][ T1362] RIP: 0010:hsr_dev_xmit+0x34/0x90 [hsr]
[  390.883528][ T1362] Code: 48 8d be 00 0c 00 00 be 04 00 00 00 48 83 ec 08 e8 21 be ff ff 48 8d 78 10 48 ba 00 00 00 0b
[  390.887020][ T1362] RSP: 0018:ffff888045507058 EFLAGS: 00010202
[  390.888067][ T1362] RAX: 0000000000000000 RBX: ffff88804a5d0cc0 RCX: 0000000000000002
[  390.889390][ T1362] RDX: dffffc0000000000 RSI: 0000000000000004 RDI: 0000000000000010
[  390.890525][ T1362] RBP: ffff88804a5d0cc0 R08: ffffed100d9c0d5d R09: 0000000000000001
[  390.891527][ T1362] R10: 0000000000000001 R11: ffffed100d9c0d5c R12: ffff888063bac000
[  390.893637][ T1362] R13: ffff888063bac088 R14: 0000000000000000 R15: ffff88806428fa00
[  390.900829][ T1362] FS:  00007fa5a5f40740(0000) GS:ffff88806cc00000(0000) knlGS:0000000000000000
[  390.908566][ T1362] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  390.909280][ T1362] CR2: 0000555eaf8cef58 CR3: 000000005c8ec002 CR4: 00000000000606e0
[  390.910070][ T1362] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  390.910899][ T1362] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  390.911722][ T1362] Call Trace:
[  390.912105][ T1362]  dev_hard_start_xmit+0x160/0x740
[  390.912640][ T1362]  __dev_queue_xmit+0x1961/0x2e10
[  390.913148][ T1362]  ? check_object+0xaf/0x260
[  390.913630][ T1362]  ? __alloc_skb+0xb9/0x500
[  390.914088][ T1362]  ? init_object+0x6b/0x80
[  390.914558][ T1362]  ? netdev_core_pick_tx+0x2e0/0x2e0
[  390.915085][ T1362]  ? __alloc_skb+0xb9/0x500
[  390.915588][ T1362]  ? rcu_read_lock_sched_held+0x90/0xc0
[  390.916182][ T1362]  ? rcu_read_lock_bh_held+0xa0/0xa0
[  390.916742][ T1362]  ? kasan_unpoison_shadow+0x30/0x40
[  390.917276][ T1362]  ? __kasan_kmalloc.constprop.4+0xa0/0xd0
[  390.924192][ T1362]  ? __kmalloc_node_track_caller+0x3a8/0x3f0
[  390.924902][ T1362]  ? __kasan_kmalloc.constprop.4+0xa0/0xd0
[  390.925662][ T1362]  ? __kmalloc_reserve.isra.46+0x2e/0xb0
[  390.926398][ T1362]  ? memset+0x1f/0x40
[  390.926904][ T1362]  ? __alloc_skb+0x317/0x500
[  390.927492][ T1362]  ? arp_xmit+0xca/0x2c0
[  390.928050][ T1362]  arp_xmit+0xca/0x2c0
[  390.928576][ T1362]  ? arp_error_report+0x150/0x150
[  390.929209][ T1362]  ? eth_header+0x1b5/0x200
[  390.929781][ T1362]  ? memset+0x1f/0x40
[  390.930288][ T1362]  ? arp_create+0x616/0x780
[  390.930857][ T1362]  arp_send_dst.part.16+0x124/0x180
[  390.931524][ T1362]  ? arp_xmit+0x2c0/0x2c0
[  390.932088][ T1362]  arp_solicit+0x8cf/0xfb0
[  390.932654][ T1362]  ? lock_downgrade+0x6e0/0x6e0
[  390.933269][ T1362]  ? arp_send+0x90/0x90
[  390.933803][ T1362]  neigh_probe+0xaf/0xf0
[ ... ]

Fixes: 311633b60406 ("hsr: switch ->dellink() to ->ndo_uninit()")
Signed-off-by: Taehee Yoo <ap420073@...il.com>
---
 net/hsr/hsr_device.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index f509b495451a..e3871491960c 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -226,9 +226,16 @@ static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct hsr_priv *hsr = netdev_priv(dev);
 	struct hsr_port *master;
 
+	rcu_read_lock();
 	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
-	skb->dev = master->dev;
-	hsr_forward_skb(skb, master);
+	if (master) {
+		skb->dev = master->dev;
+		hsr_forward_skb(skb, master);
+	} else {
+		atomic_long_inc(&dev->tx_dropped);
+		dev_kfree_skb_any(skb);
+	}
+	rcu_read_unlock();
 	return NETDEV_TX_OK;
 }
 
-- 
2.17.1