On 15.07.22 10:47, Vincent Mailhol wrote:
On Fri. 15 Jul. 2022 at 16:37, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
But that example is a bit convoluted. That's why I wrote in my
previous message that I saw no killer arguments against it.
It just feels wrong to have a pointer to a struct canxl_frame that's
only backed half by memory.
I follow you on this feeling. But have no concrete bad examples of how
the data[CANXL_MAX_DLEN] could be misused.
I haven't followed the flex array discussion
in great detail, but it opens a whole class of errors if arbitrary
kernel memory can be accessed with struct canxl_frame::data using
seemingly valid array indices.
The point in the previous discussion is that the kernel should always
crop the returned frames. If implemented correctly, such memory leaks
should not occur.
I prepared a V3 patch set for that discussion.
It ended up with the fixed struct canxl_frame analogue to the
CAN[fd]_frame's.
So whenever we have an enabled CAN XL netdevice we create a sk_buff in
raw.c which can potentially hold a struct canxl_frame.
Even when we send a CAN/CANFD frame to that CAN XL interface.
The distinction inside the kernel remains with skb->len being
CAN[|FD|XL]_MTU.
But the CAN_RAW socket read/write operations can be switched to a
dynamic (cropped) size or to a complete sizeof(struct canxl_frame) via
enhanced sockopt().
As the sk_buffs are only allocated once and are not copied inside the
kernel there should be no remarkable drawbacks whether we allocate
CAN_MTU skbs or CANXL_MTU skbs.
AFAICS both skbs will fit into a single page.
Best regards,
Oliver
