Hi Marc,
On 30/07/2019 09.33, Marc Kleine-Budde wrote:
On 7/26/19 3:18 PM, Oliver Hartkopp wrote:
In fact the only point where can_len2dlc() is active is when the CAN
frame is sent with a real CAN (FD) controller.
Maybe it would make sense to create a special flag for the can-gw
modifications like CGW_FLAGS_CAN_FD_SANITIZE_LEN.
My point is, it makes no sense to do any CAN-FD related len sanitisation
in the can-gw, as the user will not see any strange length.
So we could point out that the CAN FD length information is manipulated
and that we offer to round the values or not.
What I suggest instead is to do len sanitisation in the vcan driver, so
that the problem I illustrated above will be fixed.
Another path we have to think about is the loop back of CAN-FD frames, I
think they are not sanitised as well.
The problem in loopback and vcan is similar.
But the issues that rises up when sanitizing the FD length value in
loopback/vcan/af_can -> we start to fiddle inside the skb data.
When we do so, we need to skb_copy() the skb instead of working on
clones to prevent race conditions inside the data, see:
https://elixir.bootlin.com/linux/v5.2.6/source/net/can/gw.c#L387
skb_copy would have a performance impact and it would trigger a big
rewriting of the current code. Don't know if it's worth that.
I tend to sanitize the CAN FD length values when they are introduced
into the system (CAN_RAW, CAN_BCM, CAN FD drivers*) and when they are
modified (CAN_GW).
* = already done
My idea was to fix things up when someone manipulates the CAN or CAN FD
length field with can-gw.
Or do you think I should just omit it and let the data flow as-is?
Best,
Oliver
