Commit 2f0382a7 authored by Marc Kleine-Budde's avatar Marc Kleine-Budde
Browse files

Merge patch series "can: dev: fix can_restart() and replace BUG_ON() by error handling" 

Marc Kleine-Budde <mkl@pengutronix.de> says:

There are 2 BUG_ON() in the CAN dev helpers. During the update/test of
the at91_can driver to rx-offload the one in can_restart() was
triggered, due to a race condition in can_restart() and a hardware
limitation of the at91_can IP core.

This series fixes the race condition, replaces BUG_ON() with an error
message, and does some cleanup. Finally, the BUG_ON() in
can_put_echo_skb() is also replaced with error handling.

Changes in v2:
- 4/5: move "Restarted" debug message and stats after successful restart (Thanks Vincent)
- Link to v1: https://lore.kernel.org/all/20231004-can-dev-fix-can-restart-v1-0-2e52899eaaf5@pengutronix.de

Link: https://lore.kernel.org/all/20231005-can-dev-fix-can-restart-v2-0-91b5c1fd922c@pengutronix.de


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parents 3b933349 6411959c

drivers/net/can/dev/dev.c

+14 −15
Original line number Diff line number Diff line
@@ -132,7 +132,8 @@ static void can_restart(struct net_device *dev) 
	struct can_frame *cf;

	int err;



	BUG_ON(netif_carrier_ok(dev));

	if (netif_carrier_ok(dev))

		netdev_err(dev, "Attempt to restart for bus-off recovery, but carrier is OK?\n");



	/* No synchronization needed because the device is bus-off and

	 * no messages can come in or go out.
@@ -141,23 +142,21 @@ static void can_restart(struct net_device *dev) 


	/* send restart message upstream */

	skb = alloc_can_err_skb(dev, &cf);

	if (!skb)

		goto restart;



	if (skb) {

		cf->can_id |= CAN_ERR_RESTARTED;



		netif_rx(skb);



restart:

	netdev_dbg(dev, "restarted\n");

	priv->can_stats.restarts++;

	}



	/* Now restart the device */

	err = priv->do_set_mode(dev, CAN_MODE_START);



	netif_carrier_on(dev);

	if (err)

		netdev_err(dev, "Error %d during restart", err);

	err = priv->do_set_mode(dev, CAN_MODE_START);

	if (err) {

		netdev_err(dev, "Restart failed, error %pe\n", ERR_PTR(err));

		netif_carrier_off(dev);

	} else {

		netdev_dbg(dev, "Restarted\n");

		priv->can_stats.restarts++;

	}

}



static void can_restart_work(struct work_struct *work)

drivers/net/can/dev/skb.c

+5 −1
Original line number Diff line number Diff line
@@ -49,7 +49,11 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev, 
{

	struct can_priv *priv = netdev_priv(dev);



	BUG_ON(idx >= priv->echo_skb_max);

	if (idx >= priv->echo_skb_max) {

		netdev_err(dev, "%s: BUG! Trying to access can_priv::echo_skb out of bounds (%u/max %u)\n",

			   __func__, idx, priv->echo_skb_max);

		return -EINVAL;

	}



	/* check flag whether this packet has to be looped back */

	if (!(dev->flags & IFF_ECHO) ||