On 27/02/2020 05.23, David Miller wrote:
From: Marc Kleine-Budde mkl@pengutronix.de Date: Tue, 25 Feb 2020 21:32:41 +0100
On 1/30/20 2:30 PM, Oliver Hartkopp wrote:
Since commit 8df9ffb888c ("can: make use of preallocated can_ml_priv for per device struct can_dev_rcv_lists") the device specific CAN receive filter lists are stored in netdev_priv() and dev->ml_priv points to these filters.
In the bug report Syzkaller enslaved a vxcan1 CAN device and accessed the bonding device with a PF_CAN socket which lead to a crash due to an access of an unhandled bond_dev->ml_priv pointer.
Deny to enslave CAN devices by the bonding driver as the resulting bond_dev pretends to be a CAN device by copying dev->type without really being one.
Reported-by: syzbot+c3ea30e1e2485573f953@syzkaller.appspotmail.com Fixes: 8df9ffb888c ("can: make use of preallocated can_ml_priv for per device struct can_dev_rcv_lists") Cc: linux-stable stable@vger.kernel.org # >= v5.4 Signed-off-by: Oliver Hartkopp socketcan@hartkopp.net
Acked-by: Marc Kleine-Budde mkl@pengutronix.de
What's the preferred to upstream this? I could take this via the linux-can tree.
What I don't get is why the PF_CAN is blindly dereferencing a device assuming what is behind bond_dev->ml_priv.
If it assumes a device it access is CAN then it should check the device by comparing the netdev_ops or via some other means.
Yes we do.
This restriction seems arbitrary.
Since commit 8df9ffb888c the data structures for the CAN filters have been moved from net/can/af_can.c into netdev->ml_priv.
PF_CAN only works with CAN interfaces and therefore always checks dev->type to be ARPHRD_CAN before accessing netdev->ml_priv.
Bonding and Team driver copy most of the device data structures to create bonding/team devices. They copy dev->type but *not* dev->ml_priv. That leads to the problematic ml_priv access after passing the dev->type check ...
I don't know yet whether it makes sense to have CAN bonding/team devices. But if so we would need some more investigation. For now disabling CAN interfaces for bonding/team devices seems to be reasonable.
Regards, Oliver