mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	can: netlink: can_changelink(): fix NULL pointer deref of struct can_priv::do_set_mode
Andrei Lalaev reported a NULL pointer deref when a CAN device is
restarted from Bus Off and the driver does not implement the struct
can_priv::do_set_mode callback.
There are 2 code path that call struct can_priv::do_set_mode:
- directly by a manual restart from the user space, via
  can_changelink()
- delayed automatic restart after bus off (deactivated by default)
To prevent the NULL pointer deference, refuse a manual restart or
configure the automatic restart delay in can_changelink() and report
the error via extack to user space.
As an additional safety measure let can_restart() return an error if
can_priv::do_set_mode is not set instead of dereferencing it
unchecked.
Reported-by: Andrei Lalaev <andrey.lalaev@gmail.com>
Closes: https://lore.kernel.org/all/20250714175520.307467-1-andrey.lalaev@gmail.com
Fixes: 39549eef35 ("can: CAN Network device driver and Netlink interface")
Link: https://patch.msgid.link/20250718-fix-nullptr-deref-do_set_mode-v1-1-0b520097bb96@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
			
			
This commit is contained in:
		
							parent
							
								
									b03f15c019
								
							
						
					
					
						commit
						c1f3f9797c
					
				
					 2 changed files with 21 additions and 3 deletions
				
			
		| 
						 | 
					@ -145,13 +145,16 @@ void can_change_state(struct net_device *dev, struct can_frame *cf,
 | 
				
			||||||
EXPORT_SYMBOL_GPL(can_change_state);
 | 
					EXPORT_SYMBOL_GPL(can_change_state);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
/* CAN device restart for bus-off recovery */
 | 
					/* CAN device restart for bus-off recovery */
 | 
				
			||||||
static void can_restart(struct net_device *dev)
 | 
					static int can_restart(struct net_device *dev)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	struct can_priv *priv = netdev_priv(dev);
 | 
						struct can_priv *priv = netdev_priv(dev);
 | 
				
			||||||
	struct sk_buff *skb;
 | 
						struct sk_buff *skb;
 | 
				
			||||||
	struct can_frame *cf;
 | 
						struct can_frame *cf;
 | 
				
			||||||
	int err;
 | 
						int err;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						if (!priv->do_set_mode)
 | 
				
			||||||
 | 
							return -EOPNOTSUPP;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (netif_carrier_ok(dev))
 | 
						if (netif_carrier_ok(dev))
 | 
				
			||||||
		netdev_err(dev, "Attempt to restart for bus-off recovery, but carrier is OK?\n");
 | 
							netdev_err(dev, "Attempt to restart for bus-off recovery, but carrier is OK?\n");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -173,10 +176,14 @@ static void can_restart(struct net_device *dev)
 | 
				
			||||||
	if (err) {
 | 
						if (err) {
 | 
				
			||||||
		netdev_err(dev, "Restart failed, error %pe\n", ERR_PTR(err));
 | 
							netdev_err(dev, "Restart failed, error %pe\n", ERR_PTR(err));
 | 
				
			||||||
		netif_carrier_off(dev);
 | 
							netif_carrier_off(dev);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
							return err;
 | 
				
			||||||
	} else {
 | 
						} else {
 | 
				
			||||||
		netdev_dbg(dev, "Restarted\n");
 | 
							netdev_dbg(dev, "Restarted\n");
 | 
				
			||||||
		priv->can_stats.restarts++;
 | 
							priv->can_stats.restarts++;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						return 0;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static void can_restart_work(struct work_struct *work)
 | 
					static void can_restart_work(struct work_struct *work)
 | 
				
			||||||
| 
						 | 
					@ -201,9 +208,8 @@ int can_restart_now(struct net_device *dev)
 | 
				
			||||||
		return -EBUSY;
 | 
							return -EBUSY;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	cancel_delayed_work_sync(&priv->restart_work);
 | 
						cancel_delayed_work_sync(&priv->restart_work);
 | 
				
			||||||
	can_restart(dev);
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return 0;
 | 
						return can_restart(dev);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
/* CAN bus-off
 | 
					/* CAN bus-off
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -285,6 +285,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (data[IFLA_CAN_RESTART_MS]) {
 | 
						if (data[IFLA_CAN_RESTART_MS]) {
 | 
				
			||||||
 | 
							if (!priv->do_set_mode) {
 | 
				
			||||||
 | 
								NL_SET_ERR_MSG(extack,
 | 
				
			||||||
 | 
									       "Device doesn't support restart from Bus Off");
 | 
				
			||||||
 | 
								return -EOPNOTSUPP;
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		/* Do not allow changing restart delay while running */
 | 
							/* Do not allow changing restart delay while running */
 | 
				
			||||||
		if (dev->flags & IFF_UP)
 | 
							if (dev->flags & IFF_UP)
 | 
				
			||||||
			return -EBUSY;
 | 
								return -EBUSY;
 | 
				
			||||||
| 
						 | 
					@ -292,6 +298,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (data[IFLA_CAN_RESTART]) {
 | 
						if (data[IFLA_CAN_RESTART]) {
 | 
				
			||||||
 | 
							if (!priv->do_set_mode) {
 | 
				
			||||||
 | 
								NL_SET_ERR_MSG(extack,
 | 
				
			||||||
 | 
									       "Device doesn't support restart from Bus Off");
 | 
				
			||||||
 | 
								return -EOPNOTSUPP;
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		/* Do not allow a restart while not running */
 | 
							/* Do not allow a restart while not running */
 | 
				
			||||||
		if (!(dev->flags & IFF_UP))
 | 
							if (!(dev->flags & IFF_UP))
 | 
				
			||||||
			return -EINVAL;
 | 
								return -EINVAL;
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue