Re: [PATCH] bonding: refuse to enslave CAN devices

[Jay Vosburgh <jv@jvosburgh.net>]

Oliver Hartkopp <socketcan@hartkopp.net> wrote:

>syzbot reported a kernel paging request crash in
>can_rx_unregister() inside net/can/af_can.c. The crash occurs
>because a virtual CAN device (vxcan) is being enslaved to a
>bonding master.
>
>During the enslavement process, the bonding driver mutates
>and modifies the network device states to fit an Ethernet-like
>aggregation model. However, CAN devices operate on a completely
>different Layer 2 architecture, relying on the CAN mid-layer
>private data structure (can_ml_priv) instead of standard
>Ethernet structures. Since bonding does not initialize or
>maintain these CAN structures, subsequent operations on the
>half-enslaved interface (such as closing associated sockets
>via isotp_release) lead to a null-pointer dereference when
>accessing the CAN receiver lists.
>
>Bonding CAN interfaces is architecturally invalid as CAN lacks
>MAC addresses, ARP capabilities, and standard Ethernet
>link-layer mechanisms. While generic loopback devices are
>blocked globally in net/core/dev.c, virtual CAN devices
>bypass this check because they do not carry the IFF_LOOPBACK
>flag, despite acting as local software-loopbacks.
>
>Fix this by explicitly blocking network devices of type
>ARPHRD_CAN from being enslaved at the very beginning of
>bond_enslave(). This prevents illegal state mutations,
>eliminates the resulting KASAN crashes, and avoids potential
>memory leaks from incomplete socket cleanups.
>
>As the CAN support has been added a long time after bonding
>the Fixes-tag points to the introduction of ARPHRD_CAN that
>would have needed a specific handling in bonding_main.c.
>
>Fixes: cd05acfe65ed ("[CAN]: Allocate protocol numbers for PF_CAN")
>Reported-by: syzbot+8ed98cbd0161632bce95@syzkaller.appspotmail.com
>Closes: https://syzkaller.appspot.com/bug?extid=8ed98cbd0161632bce95
>Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

Acked-by: Jay Vosburgh <jv@jvosburgh.net>

>---
> drivers/net/bonding/bond_main.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index af82a3df2c5d..82e779f7916b 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1888,10 +1888,16 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> 	const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
> 	struct slave *new_slave = NULL, *prev_slave;
> 	struct sockaddr_storage ss;
> 	int res = 0, i;
> 
>+	if (slave_dev->type == ARPHRD_CAN) {
>+		BOND_NL_ERR(bond_dev, extack,
>+			    "CAN devices cannot be enslaved");
>+		return -EPERM;
>+	}
>+
> 	if (slave_dev->flags & IFF_MASTER &&
> 	    !netif_is_bond_master(slave_dev)) {
> 		BOND_NL_ERR(bond_dev, extack,
> 			    "Device type (master device) cannot be enslaved");
> 		return -EPERM;
>
>---
>base-commit: d60ec36cab338dfe2ae40d73e9c8d6c4af70d2b8
>change-id: 20260526-bonding-candev-f4a0cf2eee9b
>
>Best regards,
>--  
>Oliver Hartkopp <socketcan@hartkopp.net>

---
	-Jay Vosburgh, jv@jvosburgh.net