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

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 12.10.26 15:09, Marc Kleine-Budde wrote:
> On 12.10.2026 14:58:09, Oliver Hartkopp wrote:
>>
>>
>> On 10.10.26 16:02, netdev-bot+sashiko@kernel.org wrote:
>>> 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
>>>
>>>> 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
>>>
>>> [ ... ]
>>>
>>>> @@ -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/

Best regards,
Oliver