On 7/26/19 3:18 PM, Oliver Hartkopp wrote: > On 25/07/2019 16.45, Marc Kleine-Budde wrote: > >> I don't know if this is possible with can-gw, but can you do other >> calculations based on the len, so you might need the sanitized length. > > You can to AND/OR/XOR/SET operations but no arithmetic calculations. Ok >> Every user can generate such frames and the drivers have to deal with it >> anyways. >> >>> Neither the CAN controller >> >> In the TX path the CAN controller driver will translate the len into the >> dlc, so the CAN _controller_ will not see the wrong length of e.g. 13. >> >>> nor the user reading from a socket should ever get an invalid CAN FD >>> frame. >> >> What happens if the user sends a CAN FD frame with len == 13? All hw >> drivers will convert this into a DLC. > > Right. > >> What about the loopback'ed frames? >> I don't have CAN FD hardware, so I cannot test. >> What about vcan? > > They all use can_dropped_invalid_skb() which only checks the length > boundary of 8 resp. 64 byte. > > There's no check whether the CAN FD specific length values are met. > >> vcan doesn't do the len -> dlc -> len conversion: >> >> I manupulated the cansend to send a CANFD frame with the non sanitized len: >> > > (..) > >> Which is then displayed as: >> >>> vcan0 5A1 [10] 11 22 33 44 55 66 77 88 99 AA >> >> Do we need to sanitize the CAN FD frames somewhere in the stack? Note >> you can completely bypass the PF_CAN and insert the frames into the >> packet scheduler so they end up unchanged in the driver. > > Right. > > 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. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature
