On 14.11.2024 21:35:07, Vincent Mailhol wrote: > On 14/11/2024 at 18:57, Dan Carpenter wrote: > > On Thu, Nov 14, 2024 at 06:34:49PM +0900, Vincent Mailhol wrote: > > > Hi Dan, > > > > > > On 14/11/2024 at 18:03, Dan Carpenter wrote: > > > > This code is printing hex values to the &local_txbuf buffer and it's > > > > using the snprintf() function to try prevent buffer overflows. The > > > > problem is that it's not passing the correct limit to the snprintf() > > > > function so the limit doesn't do anything. On each iteration we print > > > > two digits so the remaining size should also decrease by two, but > > > > instead it passes the sizeof() the entire buffer each time. > > > > > > > > If the frame->len were too long it would result in a buffer overflow. > > > > > > But, can frame->len be too long? Classical CAN frame maximum length is 8 > > > bytes. And I do not see a path for a malformed frame to reach this part of > > > the driver. > > > > > > If such a path exists, I think this should be explained. Else, I am just not > > > sure if this needs a Fixes: tag. > > I confirmed the CAN frame length is correctly checked. > > The only way to trigger that snprintf() with the wrong size is if > CAN327_TX_DO_CAN_DATA is set, which only occurs in can327_send_frame(). And > the only caller of can327_send_frame() is can327_netdev_start_xmit(). > > can327_netdev_start_xmit() calls can_dev_dropped_skb() which in turn calls > can_dropped_invalid_skb() which goes to can_is_can_skb() which finally > checks that cf->len is not bigger than CAN_MAX_DLEN (i.e. 8 bytes). > > So indeed, no buffer overflow can occur here. > > > Even when bugs don't affect runtime we still assign a Fixes tag, but we don't > > CC stable. There is no way that passing the wrong size was intentional. > > Got it. Thanks for the explanation, now it makes sense to keep the Fixes: > tag. Should we take the patch as it is? regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
