* [PATCH] net: can: esd_usb2: check index of array before accessing
@ 2013-07-03 8:33 Marc Kleine-Budde
2013-07-03 10:00 ` Matthias Fuchs
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-07-03 8:33 UTC (permalink / raw)
To: linux-can; +Cc: Maximilian Schneider, Matthias Fuchs, Marc Kleine-Budde
From: Maximilian Schneider <max@schneidersoft.net>
The esd_usb2_read_bulk_callback() function is parsing the data that comes from
the USB CAN adapter. One datum is used as an index to access the dev->nets[]
array. This patch adds the missing bounds checking.
Cc: Matthias Fuchs <matthias.fuchs@esd.eu>
Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Hello,
Maximilian, I've added a more detailed patch description and the error message
printing. What do you think.
Marc
drivers/net/can/usb/esd_usb2.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index 6aa7b32..ac6177d 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -412,10 +412,20 @@ static void esd_usb2_read_bulk_callback(struct urb *urb)
switch (msg->msg.hdr.cmd) {
case CMD_CAN_RX:
+ if (msg->msg.rx.net >= dev->net_count) {
+ dev_err(dev->udev->dev.parent, "format error\n");
+ break;
+ }
+
esd_usb2_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
break;
case CMD_CAN_TX:
+ if (msg->msg.txdone.net >= dev->net_count) {
+ dev_err(dev->udev->dev.parent, "format error\n");
+ break;
+ }
+
esd_usb2_tx_done_msg(dev->nets[msg->msg.txdone.net],
msg);
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net: can: esd_usb2: check index of array before accessing
2013-07-03 8:33 [PATCH] net: can: esd_usb2: check index of array before accessing Marc Kleine-Budde
@ 2013-07-03 10:00 ` Matthias Fuchs
2013-07-03 10:04 ` Marc Kleine-Budde
2013-07-03 11:51 ` Max S.
0 siblings, 2 replies; 6+ messages in thread
From: Matthias Fuchs @ 2013-07-03 10:00 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can@vger.kernel.org, Maximilian Schneider
Hi,
did anybody encounter an issue with invalid net codes comming from the
device?
Well, you may add my
Acked-by: Matthias Fuchs <matthias.fuchs@esd.eu>
Matthias
On 03.07.2013 10:33, Marc Kleine-Budde wrote:
> From: Maximilian Schneider <max@schneidersoft.net>
>
> The esd_usb2_read_bulk_callback() function is parsing the data that comes from
> the USB CAN adapter. One datum is used as an index to access the dev->nets[]
> array. This patch adds the missing bounds checking.
>
> Cc: Matthias Fuchs <matthias.fuchs@esd.eu>
> Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> Hello,
>
> Maximilian, I've added a more detailed patch description and the error message
> printing. What do you think.
>
> Marc
>
> drivers/net/can/usb/esd_usb2.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
> index 6aa7b32..ac6177d 100644
> --- a/drivers/net/can/usb/esd_usb2.c
> +++ b/drivers/net/can/usb/esd_usb2.c
> @@ -412,10 +412,20 @@ static void esd_usb2_read_bulk_callback(struct urb *urb)
>
> switch (msg->msg.hdr.cmd) {
> case CMD_CAN_RX:
> + if (msg->msg.rx.net >= dev->net_count) {
> + dev_err(dev->udev->dev.parent, "format error\n");
> + break;
> + }
> +
> esd_usb2_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
> break;
>
> case CMD_CAN_TX:
> + if (msg->msg.txdone.net >= dev->net_count) {
> + dev_err(dev->udev->dev.parent, "format error\n");
> + break;
> + }
> +
> esd_usb2_tx_done_msg(dev->nets[msg->msg.txdone.net],
> msg);
> break;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: can: esd_usb2: check index of array before accessing
2013-07-03 10:00 ` Matthias Fuchs
@ 2013-07-03 10:04 ` Marc Kleine-Budde
2013-07-03 11:51 ` Max S.
1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-07-03 10:04 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: linux-can@vger.kernel.org, Maximilian Schneider
[-- Attachment #1: Type: text/plain, Size: 688 bytes --]
On 07/03/2013 12:00 PM, Matthias Fuchs wrote:
> did anybody encounter an issue with invalid net codes comming from the
> device?
No, but we should not trust data coming from the "outside" into the
kernel to be valid. I don't want to have a driver in the CAN subsystem
that may be used to exploit the kernel.
> Well, you may add my
> Acked-by: Matthias Fuchs <matthias.fuchs@esd.eu>
Thanks,
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: can: esd_usb2: check index of array before accessing
2013-07-03 10:00 ` Matthias Fuchs
2013-07-03 10:04 ` Marc Kleine-Budde
@ 2013-07-03 11:51 ` Max S.
2013-07-03 11:51 ` Marc Kleine-Budde
[not found] ` <51D41E53.2030900@esd.eu>
1 sibling, 2 replies; 6+ messages in thread
From: Max S. @ 2013-07-03 11:51 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: Marc Kleine-Budde, linux-can@vger.kernel.org
On Wed, 2013-07-03 at 12:00 +0200, Matthias Fuchs wrote:
> Hi,
>
> did anybody encounter an issue with invalid net codes comming from the
> device?
Technically one could write a usb device that sends a bad
netcode/crafted message.
through
esd_usb2_read_bulk_callback()
esd_usb2_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
...
esd_usb2_rx_event()
...
u8 state = msg->msg.rx.data[0];
...
priv->old_state = state;
one could write any u8 memory value to <255 past dev->nets.
... I think.
>
> Well, you may add my
>
> Acked-by: Matthias Fuchs <matthias.fuchs@esd.eu>
>
> Matthias
>
> On 03.07.2013 10:33, Marc Kleine-Budde wrote:
> > From: Maximilian Schneider <max@schneidersoft.net>
> >
> > The esd_usb2_read_bulk_callback() function is parsing the data that comes from
> > the USB CAN adapter. One datum is used as an index to access the dev->nets[]
> > array. This patch adds the missing bounds checking.
> >
> > Cc: Matthias Fuchs <matthias.fuchs@esd.eu>
> > Signed-off-by: Maximilian Schneider <max@schneidersoft.net>
:D
> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> > ---
> > Hello,
> >
> > Maximilian, I've added a more detailed patch description and the error message
> > printing. What do you think.
Looks good.
regards,
Max Schneider
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: can: esd_usb2: check index of array before accessing
2013-07-03 11:51 ` Max S.
@ 2013-07-03 11:51 ` Marc Kleine-Budde
[not found] ` <51D41E53.2030900@esd.eu>
1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-07-03 11:51 UTC (permalink / raw)
To: Max S.; +Cc: Matthias Fuchs, linux-can@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On 07/03/2013 01:51 PM, Max S. wrote:
> On Wed, 2013-07-03 at 12:00 +0200, Matthias Fuchs wrote:
>> Hi,
>>
>> did anybody encounter an issue with invalid net codes comming from the
>> device?
> Technically one could write a usb device that sends a bad
> netcode/crafted message.
>
> through
> esd_usb2_read_bulk_callback()
> esd_usb2_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
> ...
> esd_usb2_rx_event()
> ...
> u8 state = msg->msg.rx.data[0];
> ...
> priv->old_state = state;
>
> one could write any u8 memory value to <255 past dev->nets.
>
> ... I think.
Yes or crash the kernel by accessing bad memory, better safe then sorry.
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread[parent not found: <51D41E53.2030900@esd.eu>]
* Re: [PATCH] net: can: esd_usb2: check index of array before accessing
[not found] ` <51D41E53.2030900@esd.eu>
@ 2013-07-03 13:04 ` Marc Kleine-Budde
0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2013-07-03 13:04 UTC (permalink / raw)
To: Matthias Fuchs; +Cc: Max S., linux-can@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 766 bytes --]
On 07/03/2013 02:51 PM, Matthias Fuchs wrote:
> On 03.07.2013 13:51, Max S. wrote:
>> On Wed, 2013-07-03 at 12:00 +0200, Matthias Fuchs wrote:
>>> Hi,
>>>
>>> did anybody encounter an issue with invalid net codes comming from the
>>> device?
>> Technically one could write a usb device that sends a bad
>> netcode/crafted message.
>
> Yes of course. I only asked if somebody observed an issue with the real
> device.
There are no known issues with the real device.
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-03 13:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-03 8:33 [PATCH] net: can: esd_usb2: check index of array before accessing Marc Kleine-Budde
2013-07-03 10:00 ` Matthias Fuchs
2013-07-03 10:04 ` Marc Kleine-Budde
2013-07-03 11:51 ` Max S.
2013-07-03 11:51 ` Marc Kleine-Budde
[not found] ` <51D41E53.2030900@esd.eu>
2013-07-03 13:04 ` Marc Kleine-Budde
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.