On 10.01.2025 12:58:03, Simon Horman wrote: > On Fri, Jan 10, 2025 at 12:04:23PM +0100, Marc Kleine-Budde wrote: > > From: Jimmy Assarsson <extja@xxxxxxxxxx> > > > > Ensure statistics, error counters, and CAN state are updated consistently, > > even when alloc_can_err_skb() fails during state changes or error message > > frame reception. > > > > Signed-off-by: Jimmy Assarsson <extja@xxxxxxxxxx> > > Link: https://patch.msgid.link/20241230142645.128244-1-extja@xxxxxxxxxx > > Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> > > ... > > > diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c > > ... > > > @@ -1187,11 +1169,18 @@ static void kvaser_usb_leaf_rx_error(const struct kvaser_usb *dev, > > if (priv->can.restart_ms && > > old_state == CAN_STATE_BUS_OFF && > > new_state < CAN_STATE_BUS_OFF) { > > - cf->can_id |= CAN_ERR_RESTARTED; > > + if (cf) > > + cf->can_id |= CAN_ERR_RESTARTED; > > netif_carrier_on(priv->netdev); > > } > > } > > > > + if (!skb) { > > + stats->rx_dropped++; > > + netdev_warn(priv->netdev, "No memory left for err_skb\n"); > > + return; > > + } > > + > > switch (dev->driver_info->family) { > > case KVASER_LEAF: > > if (es->leaf.error_factor) { > > Hi Jimmy and Marc, > > The next line of this function is: > > cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT; > > Which dereferences cf. However, the check added at the top of > this hunk assumes that cf may be NULL. This doesn't seem consistent. The driver allocates the skb with: skb = alloc_can_err_skb(priv->netdev, &cf); Which in turn calls alloc_can_skb(), which finally calls: *cf = skb_put_zero(skb, sizeof(struct can_frame)); To put the cf into the skb. The newly added check "if (!skb)", takes care of skb allocation errors, so that the de-referencing of "cf" is OK after this point. regards, Marc P.S.: IIRC smatch stumbled over the same pattern in another driver a while back. Is there anything we can do about it? -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
