Re: [syzbot] [net?] KASAN: slab-out-of-bounds Read in dsa_user_changeupper

[Vladimir Oltean <olteanv@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On Tue, Jan 09, 2024 at 10:17:34AM -0800, syzbot wrote:
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in dsa_user_to_port net/dsa/user.h:58 [inline]
> BUG: KASAN: slab-out-of-bounds in dsa_user_changeupper+0x61a/0x6e0 net/dsa/user.c:2809
> Read of size 8 at addr ffff888015ebecf0 by task syz-executor278/5066
> 
> CPU: 1 PID: 5066 Comm: syz-executor278 Not tainted 6.7.0-rc6-syzkaller-01740-g9fb3dc1e9af2 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/17/2023
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
>  print_address_description mm/kasan/report.c:364 [inline]
>  print_report+0xc4/0x620 mm/kasan/report.c:475
>  kasan_report+0xda/0x110 mm/kasan/report.c:588
>  dsa_user_to_port net/dsa/user.h:58 [inline]
>  dsa_user_changeupper+0x61a/0x6e0 net/dsa/user.c:2809
>  dsa_user_netdevice_event+0xd04/0x3480 net/dsa/user.c:3345
>  notifier_call_chain+0xb6/0x3b0 kernel/notifier.c:93
>  call_netdevice_notifiers_info+0xbe/0x130 net/core/dev.c:1967
>  __netdev_upper_dev_link+0x439/0x850 net/core/dev.c:7760
>  netdev_upper_dev_link+0x92/0xc0 net/core/dev.c:7801
>  register_vlan_dev+0x396/0x940 net/8021q/vlan.c:183
>  register_vlan_device net/8021q/vlan.c:277 [inline]
>  vlan_ioctl_handler+0x8dd/0xa70 net/8021q/vlan.c:621
>  sock_ioctl+0x4bd/0x6b0 net/socket.c:1303

#syz test

[-- Attachment #2: 0001-net-dsa-fix-bad-dsa_user_to_port-calls-on-non-DSA-ne.patch --]
[-- Type: text/x-diff, Size: 3195 bytes --]

From e3b8e714f152a8fbdc72d501d3839a6350be70a0 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Tue, 9 Jan 2024 20:51:57 +0200
Subject: [PATCH] net: dsa: fix bad dsa_user_to_port() calls on non-DSA
 netdevice events

After the blamed commit, we started doing this dereference for every
NETDEV_CHANGEUPPER and NETDEV_PRECHANGEUPPER in the system.

static inline struct dsa_port *dsa_user_to_port(const struct net_device *dev)
{
	struct dsa_user_priv *p = netdev_priv(dev);

	return p->dp;
}

Syzbot unfairly blames this on commit 43a71cd66b9c ("net-device:
reorganize net_device fast path variables"), which probably changed
sizeof(struct net_device), vaguely relevant below.

static inline void *netdev_priv(const struct net_device *dev)
{
	return (char *)dev + ALIGN(sizeof(struct net_device), NETDEV_ALIGN);
}

What we see now is:

BUG: KASAN: slab-out-of-bounds in dsa_user_to_port net/dsa/user.h:58 [inline]
BUG: KASAN: slab-out-of-bounds in dsa_user_changeupper+0x61a/0x6e0 net/dsa/user.c:2809
Read of size 8 at addr ffff888015ebecf0 by task syz-executor278/5066

Call Trace:
 kasan_report+0xda/0x110 mm/kasan/report.c:588
 dsa_user_to_port net/dsa/user.h:58 [inline]
 dsa_user_changeupper+0x61a/0x6e0 net/dsa/user.c:2809
 dsa_user_netdevice_event+0xd04/0x3480 net/dsa/user.c:3345
 notifier_call_chain+0xb6/0x3b0 kernel/notifier.c:93
 call_netdevice_notifiers_info+0xbe/0x130 net/core/dev.c:1967
 __netdev_upper_dev_link+0x439/0x850 net/core/dev.c:7760
 netdev_upper_dev_link+0x92/0xc0 net/core/dev.c:7801
 register_vlan_dev+0x396/0x940 net/8021q/vlan.c:183
 register_vlan_device net/8021q/vlan.c:277 [inline]
 vlan_ioctl_handler+0x8dd/0xa70 net/8021q/vlan.c:621
 sock_ioctl+0x4bd/0x6b0 net/socket.c:1303

Full analysis of why we're seeing this just now is pending.

Fixes: 4c3f80d22b2e ("net: dsa: walk through all changeupper notifier functions")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/user.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index b738a466e2dc..b15e71cc342c 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -2806,13 +2806,14 @@ EXPORT_SYMBOL_GPL(dsa_user_dev_check);
 static int dsa_user_changeupper(struct net_device *dev,
 				struct netdev_notifier_changeupper_info *info)
 {
-	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct netlink_ext_ack *extack;
 	int err = NOTIFY_DONE;
+	struct dsa_port *dp;
 
 	if (!dsa_user_dev_check(dev))
 		return err;
 
+	dp = dsa_user_to_port(dev);
 	extack = netdev_notifier_info_to_extack(&info->info);
 
 	if (netif_is_bridge_master(info->upper_dev)) {
@@ -2865,11 +2866,13 @@ static int dsa_user_changeupper(struct net_device *dev,
 static int dsa_user_prechangeupper(struct net_device *dev,
 				   struct netdev_notifier_changeupper_info *info)
 {
-	struct dsa_port *dp = dsa_user_to_port(dev);
+	struct dsa_port *dp;
 
 	if (!dsa_user_dev_check(dev))
 		return NOTIFY_DONE;
 
+	dp = dsa_user_to_port(dev);
+
 	if (netif_is_bridge_master(info->upper_dev) && !info->linking)
 		dsa_port_pre_bridge_leave(dp, info->upper_dev);
 	else if (netif_is_lag_master(info->upper_dev) && !info->linking)
-- 
2.34.1