On 7/25/19 3:59 PM, Oliver Hartkopp wrote:
> On 25.07.19 09:04, Marc Kleine-Budde wrote:
>> On 7/24/19 8:07 PM, Oliver Hartkopp wrote:
>
>>> If the CAN FD length was 12 before the modification and the modification
>>> was to "set bit 0 in the length field" then you get 13.
>>
>> ok
>>
>>> But the length value of 13 is an illegal value for CAN FD length and can
>>> not been sent by a CAN FD controller.
>>
>> ok, but that doesn't matter, because all CAN-FD drivers must convert
>> from struct canfd_frame::len to the dlc (== their register value)
>> anyways using the can_len2dlc() function.
>>
>>> Therefore we need a round-up to get the next valid CAN FD length value
>>> (in our example it get's from 13 to 16).
>>
>> I don't think this is needed, the user space might send such packets and
>> the drivers have to deal with then anyways.
>
> Ok, maybe pointing to the driver lead to a wrong discussion.
>
> We manipulate the length info and end with a CAN FD frame which has a
> length which is unspecified in the CAN standard.
Every user can generate such frames...
> A valid CAN FD frame can not have a data length of 13.
Ok, let's put it this way: after decoding the dlc value from a real CAN
FD frame you received over the wire, the length will not be 13.
> So we fix this up after manipulation.
Why?
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.
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.
What about the loopback'ed frames?
I don't have CAN FD hardware, so I cannot test.
What about vcan?
vcan doesn't do the len -> dlc -> len conversion:
I manupulated the cansend to send a CANFD frame with the non sanitized len:
> index 8db9fd21aa99..16dbd867f493 100644
> --- a/cansend.c
> +++ b/cansend.c
> @@ -132,7 +132,7 @@ int main(int argc, char **argv)
> }
>
> /* ensure discrete CAN FD length values 0..8, 12, 16, 20, 24, 32, 64 */
> - frame.len = can_dlc2len(can_len2dlc(frame.len));
> + //frame.len = can_dlc2len(can_len2dlc(frame.len));
> }
Then I see:
> ./cansend vcan0 '5A1##0112233445566778899aa'
> write(3, "\xa1\x05\x00\x00\x0a\x00\x00\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xaa\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"..., 72) = 72
^^^
len == 0xa == 10
and the corresponding candump:
> recvmsg(3, {msg_name={sa_family=AF_CAN, sa_data="\x00\x00\x05\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"},
> msg_namelen=24->16,
> msg_iov=[{iov_base="\xa1\x05\x00\x00\x0a\x00\x00\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99\xaa\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"...,
^^^
len == 0xa == 10
> iov_len=72}], msg_iovlen=1, msg_controllen=0, msg_flags=MSG_DONTROUTE},
> 0) = 72
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.
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
