Re: [PATCH net-next 03/37] can: convert unreliable ARPHRD_CAN type checks to robust can_get_ml_priv()

On 12.10.26 15:09, Marc Kleine-Budde wrote:

On 12.10.2026 14:58:09, Oliver Hartkopp wrote:

quoted

On 10.10.26 16:02, netdev-bot+sashiko@kernel.org wrote:

quoted

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message calls the change a conversion of 'unreliable'
    ARPHRD_CAN checks to 'robust' can_get_ml_priv() checks.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but isotp_bind()
    in net/can/isotp.c ignores the return values of both can_rx_register()…

--

can: convert unreliable ARPHRD_CAN type checks to robust can_get_ml_priv()

This patch replaces the remaining dev->type != ARPHRD_CAN checks in
af_can, bcm, gw, isotp and raw with can_get_ml_priv(). The reason given is
that TUNSETLINK can switch a down TUN/TAP device to ARPHRD_CAN while its
ml_priv stays NULL.

Link: https://patch.msgid.link/20261009121115.61012-1-socketcan@hartkopp.net (local)

quoted

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 34fe3b28d576f..dc27ace43719d 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c

[ ... ]

quoted

@@ -519,7 +519,7 @@ void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id,
   	struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats;
   	struct can_dev_rcv_lists *dev_rcv_lists;
-	if (dev && dev->type != ARPHRD_CAN)
+	if (dev && !can_get_ml_priv(dev))
   		return;

[Severity: Low]
The commit message describes this change as replacing "unreliable" checks
with "robust" ones. Should it also say that the change closes a NULL
pointer dereference that a user can trigger?

Before this patch, the sequence looks like this:

    ioctl(TUNSETLINK, ARPHRD_CAN) on a down tun device
      tun->dev->type = (int) arg;      /* ml_priv stays NULL */
    isotp_bind()
      old dev->type != ARPHRD_CAN check passes
      can_rx_register() returns -ENODEV, return value ignored
      so->bound = 1; so->dev = dev;
    isotp_release() or isotp_notify()
      can_rx_unregister(net, so->dev, ...)
        old dev->type != ARPHRD_CAN guard passes
        can_dev_rcv_lists_find()
          can_ml = can_get_ml_priv(dev);   /* NULL */
          return &can_ml->dev_rcv_lists;
        hlist_for_each_entry_rcu() walks the near-NULL pointer

dev_rcv_lists sits at offset 0 of struct can_ml_priv, so the returned
pointer is near-NULL and the list walk oopses.

Triggering this only requires owning a tun device, or CAP_NET_ADMIN inside
an unprivileged user namespace.

The patch carries a Cc: stable tag. Could the message name the oops in
can_rx_unregister() (reached through isotp_release() or isotp_notify()) and
the privilege needed to trigger it?

Hi Marc,

do you want me to rephrase the commit message?

The referenced "Fixes" commit from Oleksij
4e096a18867a5a989b ("net: introduce CAN specific pointer in the struct
net_device") was introduced in Linux 5.12 but is available in stable 5.10.

please go ahead!

done!

https://lore.kernel.org/linux-can/20261012164802.94113-1-socketcan@hartkopp.net/ (local)

Best regards,
Oliver