Hi Vincent,
Dariusz thankfully reported an issue with this patch
"can: skb: drop tx skb if in listen only mode"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a6d190f8c7670
as it accesses priv->ctrlmode even on virtual CAN interfaces like vcan
and vxcan which are not created with alloc_candev() and
register_candev() and therefore do not have the can_priv data structures.
It is just an OOB read and does not crash anything but it may
potentially lead to frame losses (especially on vxcan's).
I wonder if this check for (priv->ctrlmode & CAN_CTRLMODE_LISTENONLY)
should be moved to another separate helper function
can_dropped_invalid_tx_skb()? which can then be called by every 'real'
CAN driver.
This function could perform can_dropped_invalid_skb() *and* the check
for CAN_CTRLMODE_LISTENONLY then.
The check for CAN_CTRLMODE_LISTENONLY has not really something to do
with a correct skb and for that reason can_dropped_invalid_skb() is
probably not the best place for it.
Best regards,
Oliver
On 01.11.22 12:04, Dariusz Stojaczyk wrote:
Hi Oliver, I noticed a bug in vxcan.c but I'm not a kernel developer and I'm not sure where to report it - please point me to any mailing list if required.
In vxcan_xmit() you can see
struct vxcan_priv *priv = netdev_priv(dev);
followed by:
can_dropped_invalid_skb(dev, oskb)
and can_dropped_invalid_skb() internally also uses netdev_priv, but casts it to a completely different structure:
struct can_priv *priv = netdev_priv(dev);
This gets accessed later on and does an OOB read (probably something from can_ml data)
I guess most of these can_* APIs shouldn't be used with net devices other than those registered with register_candev(). Perhaps can_dropped_invalid_skb() should be copied to vxcan.c (and vcan.c too!) and modified not to use struct can_priv.
Regards,
Darek
