Hi Marc,
On 24.07.19 15:36, Marc Kleine-Budde wrote:
@@ -425,23 +502,22 @@ static void can_can_gw_rcv(struct sk_buff *skb, void *data)
int max_len = nskb->len - offsetof(struct canfd_frame, data);
I know, this is original code...but max_len can either be 8 (for CAN
frames) or 64 (for CAN-FD frames)? Because we always have full can_frame
or canfd_frame in the skb, right? I assume a lot more will break if the
len neither 8 nor 64.
Yes. The code has been added recently for commit 0aaa81377c5a0 ("can:
gw: ensure DLC boundaries after CAN frame modification") to check the
data length after the CAN frame modification.
And yes, we only have proper CAN (FD) frames in our skbs which have a
special ethertype :-)
/* dlc may have changed, make sure it fits to the CAN frame */
- if (cf->len > max_len)
- goto out_delete;
+ if (cf->len > max_len) {
+ /* delete frame due to misconfiguration */
+ gwj->deleted_frames++;
+ kfree_skb(nskb);
+ return;
+ }
- /* check for checksum updates in classic CAN length only */
- if (gwj->mod.csumfunc.crc8) {
- if (cf->len > 8)
- goto out_delete;
+ /* ensure a valid CAN (FD) frame data length */
+ cf->len = validate_len[cf->len];
This looks strange to me, but I cannot say if I don't userstand this or
if there really is a potential problem:
- first you calculate max_len
- the cf->len > max_len is discarded
Right. In this case the frame modification leads to a length value
beyond 8/64 byte, which is then discarded.
- but then cf->len is "rounded up" via validate_len[].
What's the purpose of the last step?
Ha, good question :-)
The reason for this results from the non-linear data length code for CAN
FD data length.
For CAN you can have 0, 1, 2, 3, 4, 5, 6, 7, 8
For CAN FD it is 0, 1, 2, 3, 4, 5, 6, 7, 8, 12, 16, 20, 24, 32, 48, 64
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.
But the length value of 13 is an illegal value for CAN FD length and can
not been sent by a CAN FD controller.
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).
Regards,
Oliver
