* [PATCH net] can: peak_usb: add range checking in decode operations
@ 2020-08-13 14:06 ` Dan Carpenter
0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2020-08-13 14:06 UTC (permalink / raw)
To: Wolfgang Grandegger, Stephane Grosjean
Cc: Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
Andri Yngvason, Oliver Hartkopp, linux-can, netdev,
kernel-janitors
These values come from skb->data so Smatch considers them untrusted. I
believe Smatch is correct but I don't have a way to test this.
The usb_if->dev[] array has 2 elements but the index is in the 0-15
range without checks. The cfd->len can be up to 255 but the maximum
valid size is CANFD_MAX_DLEN (64) so that could lead to memory
corruption.
Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 48 +++++++++++++++++-----
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 47cc1ff5b88e..dee3e689b54d 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_rx_msg *rm = (struct pucan_rx_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_msg_get_channel(rm)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct canfd_frame *cfd;
struct sk_buff *skb;
const u16 rx_msg_flags = le16_to_cpu(rm->flags);
+ if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_msg_get_channel(rm)];
+ netdev = dev->netdev;
+
if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) {
/* CANFD frame case */
skb = alloc_canfd_skb(netdev, &cfd);
@@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_status_msg *sm = (struct pucan_status_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
- struct pcan_usb_fd_device *pdev =
- container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
enum can_state rx_state, tx_state;
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pucan_stmsg_get_channel(sm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
+ netdev = dev->netdev;
+
/* nothing should be sent while in BUS_OFF state */
if (dev->can.state == CAN_STATE_BUS_OFF)
return 0;
@@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_ermsg_get_channel(er)];
- struct pcan_usb_fd_device *pdev =
- container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
+ struct peak_usb_device *dev;
+
+ if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pucan_ermsg_get_channel(er)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
/* keep a trace of tx and rx error counters for later use */
pdev->bec.txerr = er->tx_err_cnt;
@@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pufd_omsg_get_channel(ov)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pufd_omsg_get_channel(ov) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pufd_omsg_get_channel(ov)];
+ netdev = dev->netdev;
+
/* allocate an skb to store the error frame */
skb = alloc_can_err_skb(netdev, &cf);
if (!skb)
@@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev,
u16 tx_msg_size, tx_msg_flags;
u8 can_dlc;
+ if (cfd->len > CANFD_MAX_DLEN)
+ return -EINVAL;
+
tx_msg_size = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4);
tx_msg->size = cpu_to_le16(tx_msg_size);
tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX);
--
2.28.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH net] can: peak_usb: add range checking in decode operations
@ 2020-08-13 14:06 ` Dan Carpenter
0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2020-08-13 14:06 UTC (permalink / raw)
To: Wolfgang Grandegger, Stephane Grosjean
Cc: Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
Andri Yngvason, Oliver Hartkopp, linux-can, netdev,
kernel-janitors
These values come from skb->data so Smatch considers them untrusted. I
believe Smatch is correct but I don't have a way to test this.
The usb_if->dev[] array has 2 elements but the index is in the 0-15
range without checks. The cfd->len can be up to 255 but the maximum
valid size is CANFD_MAX_DLEN (64) so that could lead to memory
corruption.
Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 48 +++++++++++++++++-----
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 47cc1ff5b88e..dee3e689b54d 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_rx_msg *rm = (struct pucan_rx_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_msg_get_channel(rm)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct canfd_frame *cfd;
struct sk_buff *skb;
const u16 rx_msg_flags = le16_to_cpu(rm->flags);
+ if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_msg_get_channel(rm)];
+ netdev = dev->netdev;
+
if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) {
/* CANFD frame case */
skb = alloc_canfd_skb(netdev, &cfd);
@@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_status_msg *sm = (struct pucan_status_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
- struct pcan_usb_fd_device *pdev - container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
enum can_state rx_state, tx_state;
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pucan_stmsg_get_channel(sm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
+ netdev = dev->netdev;
+
/* nothing should be sent while in BUS_OFF state */
if (dev->can.state = CAN_STATE_BUS_OFF)
return 0;
@@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_ermsg_get_channel(er)];
- struct pcan_usb_fd_device *pdev - container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
+ struct peak_usb_device *dev;
+
+ if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pucan_ermsg_get_channel(er)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
/* keep a trace of tx and rx error counters for later use */
pdev->bec.txerr = er->tx_err_cnt;
@@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pufd_omsg_get_channel(ov)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pufd_omsg_get_channel(ov) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pufd_omsg_get_channel(ov)];
+ netdev = dev->netdev;
+
/* allocate an skb to store the error frame */
skb = alloc_can_err_skb(netdev, &cf);
if (!skb)
@@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev,
u16 tx_msg_size, tx_msg_flags;
u8 can_dlc;
+ if (cfd->len > CANFD_MAX_DLEN)
+ return -EINVAL;
+
tx_msg_size = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4);
tx_msg->size = cpu_to_le16(tx_msg_size);
tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX);
--
2.28.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH net] can: peak_usb: add range checking in decode operations
2020-08-13 14:06 ` Dan Carpenter
(?)
@ 2020-10-14 13:22 ` Stéphane Grosjean
2020-10-14 17:48 ` Oliver Hartkopp
-1 siblings, 1 reply; 11+ messages in thread
From: Stéphane Grosjean @ 2020-10-14 13:22 UTC (permalink / raw)
To: Dan Carpenter
Cc: Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
Andri Yngvason, Oliver Hartkopp, linux-can, netdev,
kernel-janitors, Wolfgang Grandegger
Hello Dan,
Don't know if this patch is still relevant, but:
- there is absolutely no reason for the device firmware to provide a channel index greater than or equal to 2, because the IP core of these USB devices handles 2 channels only. Anyway, these changes are correct.
- considering the verification of the length "cfd->len" on the other hand, this one comes directly from can_send() via dev_queue_xmit() AFAIK and it seems to me that the underlying driver can assume that its value is smaller than 64.
Regards,
---
Stéphane Grosjean
PEAK-System France
132, rue André Bisiaux
F-54320 MAXEVILLE
Tél : +(33) 9.72.54.51.97
De : Dan Carpenter <dan.carpenter@oracle.com>
Envoyé : jeudi 13 août 2020 16:06
À : Wolfgang Grandegger <wg@grandegger.com>; Stéphane Grosjean <s.grosjean@peak-system.com>
Cc : Marc Kleine-Budde <mkl@pengutronix.de>; David S. Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Andri Yngvason <andri.yngvason@marel.com>; Oliver Hartkopp <socketcan@hartkopp.net>; linux-can@vger.kernel.org <linux-can@vger.kernel.org>; netdev@vger.kernel.org <netdev@vger.kernel.org>; kernel-janitors@vger.kernel.org <kernel-janitors@vger.kernel.org>
Objet : [PATCH net] can: peak_usb: add range checking in decode operations
These values come from skb->data so Smatch considers them untrusted. I
believe Smatch is correct but I don't have a way to test this.
The usb_if->dev[] array has 2 elements but the index is in the 0-15
range without checks. The cfd->len can be up to 255 but the maximum
valid size is CANFD_MAX_DLEN (64) so that could lead to memory
corruption.
Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 48 +++++++++++++++++-----
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 47cc1ff5b88e..dee3e689b54d 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_rx_msg *rm = (struct pucan_rx_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_msg_get_channel(rm)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct canfd_frame *cfd;
struct sk_buff *skb;
const u16 rx_msg_flags = le16_to_cpu(rm->flags);
+ if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_msg_get_channel(rm)];
+ netdev = dev->netdev;
+
if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) {
/* CANFD frame case */
skb = alloc_canfd_skb(netdev, &cfd);
@@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_status_msg *sm = (struct pucan_status_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
- struct pcan_usb_fd_device *pdev =
- container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
enum can_state rx_state, tx_state;
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pucan_stmsg_get_channel(sm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
+ netdev = dev->netdev;
+
/* nothing should be sent while in BUS_OFF state */
if (dev->can.state == CAN_STATE_BUS_OFF)
return 0;
@@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_ermsg_get_channel(er)];
- struct pcan_usb_fd_device *pdev =
- container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
+ struct peak_usb_device *dev;
+
+ if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pucan_ermsg_get_channel(er)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
/* keep a trace of tx and rx error counters for later use */
pdev->bec.txerr = er->tx_err_cnt;
@@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pufd_omsg_get_channel(ov)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pufd_omsg_get_channel(ov) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pufd_omsg_get_channel(ov)];
+ netdev = dev->netdev;
+
/* allocate an skb to store the error frame */
skb = alloc_can_err_skb(netdev, &cf);
if (!skb)
@@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev,
u16 tx_msg_size, tx_msg_flags;
u8 can_dlc;
+ if (cfd->len > CANFD_MAX_DLEN)
+ return -EINVAL;
+
tx_msg_size = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4);
tx_msg->size = cpu_to_le16(tx_msg_size);
tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX);
--
2.28.0
--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH net] can: peak_usb: add range checking in decode operations
2020-10-14 13:22 ` Stéphane Grosjean
@ 2020-10-14 17:48 ` Oliver Hartkopp
0 siblings, 0 replies; 11+ messages in thread
From: Oliver Hartkopp @ 2020-10-14 17:48 UTC (permalink / raw)
To: Stéphane Grosjean, Dan Carpenter
Cc: Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
Andri Yngvason, linux-can, netdev, kernel-janitors,
Wolfgang Grandegger
Hi Stephane,
On 14.10.20 15:22, Stéphane Grosjean wrote:
> Hello Dan,
>
> Don't know if this patch is still relevant, but:
>
> - there is absolutely no reason for the device firmware to provide a channel index greater than or equal to 2, because the IP core of these USB devices handles 2 channels only. Anyway, these changes are correct.
> - considering the verification of the length "cfd->len" on the other hand, this one comes directly from can_send() via dev_queue_xmit() AFAIK and it seems to me that the underlying driver can assume that its value is smaller than 64.
In fact there are many inbound checks e.g. with
can_dropped_invalid_skb() to make sure the network layer gets proper CAN
skbs (with ETH_P_CAN(FD) ethertypes).
On the outgoing path the CAN driver gets these ETH_P_CAN(FD) CAN skbs an
just copies the CAN ID and the up to 64 bytes of data from that skb.
But remember that you can also generate CAN frames via AF_PACKET sockets
which does not perform the sanity checks from can_send():
https://github.com/linux-can/can-tests/blob/master/netlayer/tst-packet.c
Copying 64 byte from the skb into an I/O attached CAN controller is
always a safe operation - but when you send the content through another
medium (e.g. USB) the length values should be checked.
Best regards,
Oliver
>
> Regards,
> ---
> Stéphane Grosjean
> PEAK-System France
> 132, rue André Bisiaux
> F-54320 MAXEVILLE
> Tél : +(33) 9.72.54.51.97
>
>
>
> De : Dan Carpenter <dan.carpenter@oracle.com>
> Envoyé : jeudi 13 août 2020 16:06
> À : Wolfgang Grandegger <wg@grandegger.com>; Stéphane Grosjean <s.grosjean@peak-system.com>
> Cc : Marc Kleine-Budde <mkl@pengutronix.de>; David S. Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Andri Yngvason <andri.yngvason@marel.com>; Oliver Hartkopp <socketcan@hartkopp.net>; linux-can@vger.kernel.org <linux-can@vger.kernel.org>; netdev@vger.kernel.org <netdev@vger.kernel.org>; kernel-janitors@vger.kernel.org <kernel-janitors@vger.kernel.org>
> Objet : [PATCH net] can: peak_usb: add range checking in decode operations
>
> These values come from skb->data so Smatch considers them untrusted. I
> believe Smatch is correct but I don't have a way to test this.
>
> The usb_if->dev[] array has 2 elements but the index is in the 0-15
> range without checks. The cfd->len can be up to 255 but the maximum
> valid size is CANFD_MAX_DLEN (64) so that could lead to memory
> corruption.
>
> Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 48 +++++++++++++++++-----
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> index 47cc1ff5b88e..dee3e689b54d 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> @@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pucan_rx_msg *rm = (struct pucan_rx_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pucan_msg_get_channel(rm)];
> - struct net_device *netdev = dev->netdev;
> + struct peak_usb_device *dev;
> + struct net_device *netdev;
> struct canfd_frame *cfd;
> struct sk_buff *skb;
> const u16 rx_msg_flags = le16_to_cpu(rm->flags);
>
> + if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev))
> + return -ENOMEM;
> +
> + dev = usb_if->dev[pucan_msg_get_channel(rm)];
> + netdev = dev->netdev;
> +
> if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) {
> /* CANFD frame case */
> skb = alloc_canfd_skb(netdev, &cfd);
> @@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pucan_status_msg *sm = (struct pucan_status_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
> - struct pcan_usb_fd_device *pdev =
> - container_of(dev, struct pcan_usb_fd_device, dev);
> + struct pcan_usb_fd_device *pdev;
> enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> enum can_state rx_state, tx_state;
> - struct net_device *netdev = dev->netdev;
> + struct peak_usb_device *dev;
> + struct net_device *netdev;
> struct can_frame *cf;
> struct sk_buff *skb;
>
> + if (pucan_stmsg_get_channel(sm) >= ARRAY_SIZE(usb_if->dev))
> + return -ENOMEM;
> +
> + dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
> + pdev = container_of(dev, struct pcan_usb_fd_device, dev);
> + netdev = dev->netdev;
> +
> /* nothing should be sent while in BUS_OFF state */
> if (dev->can.state == CAN_STATE_BUS_OFF)
> return 0;
> @@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pucan_ermsg_get_channel(er)];
> - struct pcan_usb_fd_device *pdev =
> - container_of(dev, struct pcan_usb_fd_device, dev);
> + struct pcan_usb_fd_device *pdev;
> + struct peak_usb_device *dev;
> +
> + if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev))
> + return -EINVAL;
> +
> + dev = usb_if->dev[pucan_ermsg_get_channel(er)];
> + pdev = container_of(dev, struct pcan_usb_fd_device, dev);
>
> /* keep a trace of tx and rx error counters for later use */
> pdev->bec.txerr = er->tx_err_cnt;
> @@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pufd_omsg_get_channel(ov)];
> - struct net_device *netdev = dev->netdev;
> + struct peak_usb_device *dev;
> + struct net_device *netdev;
> struct can_frame *cf;
> struct sk_buff *skb;
>
> + if (pufd_omsg_get_channel(ov) >= ARRAY_SIZE(usb_if->dev))
> + return -EINVAL;
> +
> + dev = usb_if->dev[pufd_omsg_get_channel(ov)];
> + netdev = dev->netdev;
> +
> /* allocate an skb to store the error frame */
> skb = alloc_can_err_skb(netdev, &cf);
> if (!skb)
> @@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev,
> u16 tx_msg_size, tx_msg_flags;
> u8 can_dlc;
>
> + if (cfd->len > CANFD_MAX_DLEN)
> + return -EINVAL;
> +
> tx_msg_size = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4);
> tx_msg->size = cpu_to_le16(tx_msg_size);
> tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX);
> --
> 2.28.0
>
> --
> PEAK-System Technik GmbH
> Sitz der Gesellschaft Darmstadt - HRB 9183
> Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
> Unsere Datenschutzerklaerung mit wichtigen Hinweisen
> zur Behandlung personenbezogener Daten finden Sie unter
> www.peak-system.com/Datenschutz.483.0.html
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] can: peak_usb: add range checking in decode operations
@ 2020-10-14 17:48 ` Oliver Hartkopp
0 siblings, 0 replies; 11+ messages in thread
From: Oliver Hartkopp @ 2020-10-14 17:48 UTC (permalink / raw)
To: Stéphane Grosjean, Dan Carpenter
Cc: Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
Andri Yngvason, linux-can, netdev, kernel-janitors,
Wolfgang Grandegger
Hi Stephane,
On 14.10.20 15:22, Stéphane Grosjean wrote:
> Hello Dan,
>
> Don't know if this patch is still relevant, but:
>
> - there is absolutely no reason for the device firmware to provide a channel index greater than or equal to 2, because the IP core of these USB devices handles 2 channels only. Anyway, these changes are correct.
> - considering the verification of the length "cfd->len" on the other hand, this one comes directly from can_send() via dev_queue_xmit() AFAIK and it seems to me that the underlying driver can assume that its value is smaller than 64.
In fact there are many inbound checks e.g. with
can_dropped_invalid_skb() to make sure the network layer gets proper CAN
skbs (with ETH_P_CAN(FD) ethertypes).
On the outgoing path the CAN driver gets these ETH_P_CAN(FD) CAN skbs an
just copies the CAN ID and the up to 64 bytes of data from that skb.
But remember that you can also generate CAN frames via AF_PACKET sockets
which does not perform the sanity checks from can_send():
https://github.com/linux-can/can-tests/blob/master/netlayer/tst-packet.c
Copying 64 byte from the skb into an I/O attached CAN controller is
always a safe operation - but when you send the content through another
medium (e.g. USB) the length values should be checked.
Best regards,
Oliver
>
> Regards,
> ---
> Stéphane Grosjean
> PEAK-System France
> 132, rue André Bisiaux
> F-54320 MAXEVILLE
> Tél : +(33) 9.72.54.51.97
>
>
>
> De : Dan Carpenter <dan.carpenter@oracle.com>
> Envoyé : jeudi 13 août 2020 16:06
> À : Wolfgang Grandegger <wg@grandegger.com>; Stéphane Grosjean <s.grosjean@peak-system.com>
> Cc : Marc Kleine-Budde <mkl@pengutronix.de>; David S. Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Andri Yngvason <andri.yngvason@marel.com>; Oliver Hartkopp <socketcan@hartkopp.net>; linux-can@vger.kernel.org <linux-can@vger.kernel.org>; netdev@vger.kernel.org <netdev@vger.kernel.org>; kernel-janitors@vger.kernel.org <kernel-janitors@vger.kernel.org>
> Objet : [PATCH net] can: peak_usb: add range checking in decode operations
>
> These values come from skb->data so Smatch considers them untrusted. I
> believe Smatch is correct but I don't have a way to test this.
>
> The usb_if->dev[] array has 2 elements but the index is in the 0-15
> range without checks. The cfd->len can be up to 255 but the maximum
> valid size is CANFD_MAX_DLEN (64) so that could lead to memory
> corruption.
>
> Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 48 +++++++++++++++++-----
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> index 47cc1ff5b88e..dee3e689b54d 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
> @@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pucan_rx_msg *rm = (struct pucan_rx_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pucan_msg_get_channel(rm)];
> - struct net_device *netdev = dev->netdev;
> + struct peak_usb_device *dev;
> + struct net_device *netdev;
> struct canfd_frame *cfd;
> struct sk_buff *skb;
> const u16 rx_msg_flags = le16_to_cpu(rm->flags);
>
> + if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev))
> + return -ENOMEM;
> +
> + dev = usb_if->dev[pucan_msg_get_channel(rm)];
> + netdev = dev->netdev;
> +
> if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) {
> /* CANFD frame case */
> skb = alloc_canfd_skb(netdev, &cfd);
> @@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pucan_status_msg *sm = (struct pucan_status_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
> - struct pcan_usb_fd_device *pdev > - container_of(dev, struct pcan_usb_fd_device, dev);
> + struct pcan_usb_fd_device *pdev;
> enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
> enum can_state rx_state, tx_state;
> - struct net_device *netdev = dev->netdev;
> + struct peak_usb_device *dev;
> + struct net_device *netdev;
> struct can_frame *cf;
> struct sk_buff *skb;
>
> + if (pucan_stmsg_get_channel(sm) >= ARRAY_SIZE(usb_if->dev))
> + return -ENOMEM;
> +
> + dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
> + pdev = container_of(dev, struct pcan_usb_fd_device, dev);
> + netdev = dev->netdev;
> +
> /* nothing should be sent while in BUS_OFF state */
> if (dev->can.state = CAN_STATE_BUS_OFF)
> return 0;
> @@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pucan_ermsg_get_channel(er)];
> - struct pcan_usb_fd_device *pdev > - container_of(dev, struct pcan_usb_fd_device, dev);
> + struct pcan_usb_fd_device *pdev;
> + struct peak_usb_device *dev;
> +
> + if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev))
> + return -EINVAL;
> +
> + dev = usb_if->dev[pucan_ermsg_get_channel(er)];
> + pdev = container_of(dev, struct pcan_usb_fd_device, dev);
>
> /* keep a trace of tx and rx error counters for later use */
> pdev->bec.txerr = er->tx_err_cnt;
> @@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if,
> struct pucan_msg *rx_msg)
> {
> struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_msg *)rx_msg;
> - struct peak_usb_device *dev = usb_if->dev[pufd_omsg_get_channel(ov)];
> - struct net_device *netdev = dev->netdev;
> + struct peak_usb_device *dev;
> + struct net_device *netdev;
> struct can_frame *cf;
> struct sk_buff *skb;
>
> + if (pufd_omsg_get_channel(ov) >= ARRAY_SIZE(usb_if->dev))
> + return -EINVAL;
> +
> + dev = usb_if->dev[pufd_omsg_get_channel(ov)];
> + netdev = dev->netdev;
> +
> /* allocate an skb to store the error frame */
> skb = alloc_can_err_skb(netdev, &cf);
> if (!skb)
> @@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev,
> u16 tx_msg_size, tx_msg_flags;
> u8 can_dlc;
>
> + if (cfd->len > CANFD_MAX_DLEN)
> + return -EINVAL;
> +
> tx_msg_size = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4);
> tx_msg->size = cpu_to_le16(tx_msg_size);
> tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX);
> --
> 2.28.0
>
> --
> PEAK-System Technik GmbH
> Sitz der Gesellschaft Darmstadt - HRB 9183
> Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
> Unsere Datenschutzerklaerung mit wichtigen Hinweisen
> zur Behandlung personenbezogener Daten finden Sie unter
> www.peak-system.com/Datenschutz.483.0.html
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] can: peak_usb: add range checking in decode operations
[not found] ` <VI1PR03MB5053CAE3ED35D8E9C23063F8D6030@VI1PR03MB5053.eurprd03.prod.outlook.com>
@ 2020-10-16 7:51 ` Marc Kleine-Budde
0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2020-10-16 7:51 UTC (permalink / raw)
To: Stéphane Grosjean, Oliver Hartkopp, Dan Carpenter
Cc: David S. Miller, Jakub Kicinski, Andri Yngvason, linux-can,
netdev, kernel-janitors, Wolfgang Grandegger
[-- Attachment #1.1: Type: text/plain, Size: 585 bytes --]
On 10/16/20 9:43 AM, Stéphane Grosjean wrote:
> Thank you for your detailed answer. And so? AFAIK I saw that this patch is still
> not in mainline. What needs to be done for it to be there? Should I bring an
> Ack, for example?
If you have reviewed the patch, give an Ack or Reviewed-by.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] can: peak_usb: add range checking in decode operations
@ 2020-10-16 7:51 ` Marc Kleine-Budde
0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2020-10-16 7:51 UTC (permalink / raw)
To: Stéphane Grosjean, Oliver Hartkopp, Dan Carpenter
Cc: David S. Miller, Jakub Kicinski, Andri Yngvason, linux-can,
netdev, kernel-janitors, Wolfgang Grandegger
[-- Attachment #1.1: Type: text/plain, Size: 585 bytes --]
On 10/16/20 9:43 AM, Stéphane Grosjean wrote:
> Thank you for your detailed answer. And so? AFAIK I saw that this patch is still
> not in mainline. What needs to be done for it to be there? Should I bring an
> Ack, for example?
If you have reviewed the patch, give an Ack or Reviewed-by.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH net] can: peak_usb: add range checking in decode operations
2020-08-13 14:06 ` Dan Carpenter
@ 2020-10-26 13:28 ` Stéphane Grosjean
-1 siblings, 0 replies; 11+ messages in thread
From: Stéphane Grosjean @ 2020-10-26 13:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
Andri Yngvason, Oliver Hartkopp, linux-can, netdev,
kernel-janitors, Wolfgang Grandegger
Hello Dan,
You have my Ack.
Acked-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
Stéphane Grosjean
PEAK-System France
132, rue André Bisiaux
F-54320 MAXEVILLE
Tél : +(33) 9.72.54.51.97
De : Dan Carpenter <dan.carpenter@oracle.com>
Envoyé : jeudi 13 août 2020 16:06
À : Wolfgang Grandegger <wg@grandegger.com>; Stéphane Grosjean <s.grosjean@peak-system.com>
Cc : Marc Kleine-Budde <mkl@pengutronix.de>; David S. Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Andri Yngvason <andri.yngvason@marel.com>; Oliver Hartkopp <socketcan@hartkopp.net>; linux-can@vger.kernel.org <linux-can@vger.kernel.org>; netdev@vger.kernel.org <netdev@vger.kernel.org>; kernel-janitors@vger.kernel.org <kernel-janitors@vger.kernel.org>
Objet : [PATCH net] can: peak_usb: add range checking in decode operations
These values come from skb->data so Smatch considers them untrusted. I
believe Smatch is correct but I don't have a way to test this.
The usb_if->dev[] array has 2 elements but the index is in the 0-15
range without checks. The cfd->len can be up to 255 but the maximum
valid size is CANFD_MAX_DLEN (64) so that could lead to memory
corruption.
Fixes: 0a25e1f4f185 ("can: peak_usb: add support for PEAK new CANFD USB adapters")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 48 +++++++++++++++++-----
1 file changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 47cc1ff5b88e..dee3e689b54d 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -468,12 +468,18 @@ static int pcan_usb_fd_decode_canmsg(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_rx_msg *rm = (struct pucan_rx_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_msg_get_channel(rm)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct canfd_frame *cfd;
struct sk_buff *skb;
const u16 rx_msg_flags = le16_to_cpu(rm->flags);
+ if (pucan_msg_get_channel(rm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_msg_get_channel(rm)];
+ netdev = dev->netdev;
+
if (rx_msg_flags & PUCAN_MSG_EXT_DATA_LEN) {
/* CANFD frame case */
skb = alloc_canfd_skb(netdev, &cfd);
@@ -519,15 +525,21 @@ static int pcan_usb_fd_decode_status(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_status_msg *sm = (struct pucan_status_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
- struct pcan_usb_fd_device *pdev =
- container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
enum can_state new_state = CAN_STATE_ERROR_ACTIVE;
enum can_state rx_state, tx_state;
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pucan_stmsg_get_channel(sm) >= ARRAY_SIZE(usb_if->dev))
+ return -ENOMEM;
+
+ dev = usb_if->dev[pucan_stmsg_get_channel(sm)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
+ netdev = dev->netdev;
+
/* nothing should be sent while in BUS_OFF state */
if (dev->can.state == CAN_STATE_BUS_OFF)
return 0;
@@ -579,9 +591,14 @@ static int pcan_usb_fd_decode_error(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pucan_error_msg *er = (struct pucan_error_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pucan_ermsg_get_channel(er)];
- struct pcan_usb_fd_device *pdev =
- container_of(dev, struct pcan_usb_fd_device, dev);
+ struct pcan_usb_fd_device *pdev;
+ struct peak_usb_device *dev;
+
+ if (pucan_ermsg_get_channel(er) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pucan_ermsg_get_channel(er)];
+ pdev = container_of(dev, struct pcan_usb_fd_device, dev);
/* keep a trace of tx and rx error counters for later use */
pdev->bec.txerr = er->tx_err_cnt;
@@ -595,11 +612,17 @@ static int pcan_usb_fd_decode_overrun(struct pcan_usb_fd_if *usb_if,
struct pucan_msg *rx_msg)
{
struct pcan_ufd_ovr_msg *ov = (struct pcan_ufd_ovr_msg *)rx_msg;
- struct peak_usb_device *dev = usb_if->dev[pufd_omsg_get_channel(ov)];
- struct net_device *netdev = dev->netdev;
+ struct peak_usb_device *dev;
+ struct net_device *netdev;
struct can_frame *cf;
struct sk_buff *skb;
+ if (pufd_omsg_get_channel(ov) >= ARRAY_SIZE(usb_if->dev))
+ return -EINVAL;
+
+ dev = usb_if->dev[pufd_omsg_get_channel(ov)];
+ netdev = dev->netdev;
+
/* allocate an skb to store the error frame */
skb = alloc_can_err_skb(netdev, &cf);
if (!skb)
@@ -716,6 +739,9 @@ static int pcan_usb_fd_encode_msg(struct peak_usb_device *dev,
u16 tx_msg_size, tx_msg_flags;
u8 can_dlc;
+ if (cfd->len > CANFD_MAX_DLEN)
+ return -EINVAL;
+
tx_msg_size = ALIGN(sizeof(struct pucan_tx_msg) + cfd->len, 4);
tx_msg->size = cpu_to_le16(tx_msg_size);
tx_msg->type = cpu_to_le16(PUCAN_MSG_CAN_TX);
--
2.28.0
--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH net] can: peak_usb: add range checking in decode operations
@ 2020-10-26 13:28 ` Stéphane Grosjean
0 siblings, 0 replies; 11+ messages in thread
From: Stéphane Grosjean @ 2020-10-26 13:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
Andri Yngvason, Oliver Hartkopp, linux-can, netdev,
kernel-janitors, Wolfgang Grandegger
4oCL4oCLSGVsbG8gRGFuLA0KDQpZb3UgaGF2ZSBteSBBY2suDQoNCkFja2VkLWJ5OiBTdGVwaGFu
ZSBHcm9zamVhbiA8cy5ncm9zamVhbkBwZWFrLXN5c3RlbS5jb20+DQoNCi0tLQ0KU3TDqXBoYW5l
IEdyb3NqZWFuDQpQRUFLLVN5c3RlbSBGcmFuY2UNCjEzMiwgcnVlIEFuZHLDqSBCaXNpYXV4DQpG
LTU0MzIwIE1BWEVWSUxMRQ0KVMOpbCA6ICsoMzMpIDkuNzIuNTQuNTEuOTcNCg0KDQoNCkRlIDog
RGFuIENhcnBlbnRlciA8ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29tPg0KRW52b3nDqSA6IGpldWRp
IDEzIGFvw7t0IDIwMjAgMTY6MDYNCsOAIDogV29sZmdhbmcgR3JhbmRlZ2dlciA8d2dAZ3JhbmRl
Z2dlci5jb20+OyBTdMOpcGhhbmUgR3Jvc2plYW4gPHMuZ3Jvc2plYW5AcGVhay1zeXN0ZW0uY29t
Pg0KQ2MgOiBNYXJjIEtsZWluZS1CdWRkZSA8bWtsQHBlbmd1dHJvbml4LmRlPjsgRGF2aWQgUy4g
TWlsbGVyIDxkYXZlbUBkYXZlbWxvZnQubmV0PjsgSmFrdWIgS2ljaW5za2kgPGt1YmFAa2VybmVs
Lm9yZz47IEFuZHJpIFluZ3Zhc29uIDxhbmRyaS55bmd2YXNvbkBtYXJlbC5jb20+OyBPbGl2ZXIg
SGFydGtvcHAgPHNvY2tldGNhbkBoYXJ0a29wcC5uZXQ+OyBsaW51eC1jYW5Admdlci5rZXJuZWwu
b3JnIDxsaW51eC1jYW5Admdlci5rZXJuZWwub3JnPjsgbmV0ZGV2QHZnZXIua2VybmVsLm9yZyA8
bmV0ZGV2QHZnZXIua2VybmVsLm9yZz47IGtlcm5lbC1qYW5pdG9yc0B2Z2VyLmtlcm5lbC5vcmcg
PGtlcm5lbC1qYW5pdG9yc0B2Z2VyLmtlcm5lbC5vcmc+DQpPYmpldCA6IFtQQVRDSCBuZXRdIGNh
bjogcGVha191c2I6IGFkZCByYW5nZSBjaGVja2luZyBpbiBkZWNvZGUgb3BlcmF0aW9ucw0KDQpU
aGVzZSB2YWx1ZXMgY29tZSBmcm9tIHNrYi0+ZGF0YSBzbyBTbWF0Y2ggY29uc2lkZXJzIHRoZW0g
dW50cnVzdGVkLiAgSQ0KYmVsaWV2ZSBTbWF0Y2ggaXMgY29ycmVjdCBidXQgSSBkb24ndCBoYXZl
IGEgd2F5IHRvIHRlc3QgdGhpcy4NCg0KVGhlIHVzYl9pZi0+ZGV2W10gYXJyYXkgaGFzIDIgZWxl
bWVudHMgYnV0IHRoZSBpbmRleCBpcyBpbiB0aGUgMC0xNQ0KcmFuZ2Ugd2l0aG91dCBjaGVja3Mu
ICBUaGUgY2ZkLT5sZW4gY2FuIGJlIHVwIHRvIDI1NSBidXQgdGhlIG1heGltdW0NCnZhbGlkIHNp
emUgaXMgQ0FORkRfTUFYX0RMRU4gKDY0KSBzbyB0aGF0IGNvdWxkIGxlYWQgdG8gbWVtb3J5DQpj
b3JydXB0aW9uLg0KDQpGaXhlczogMGEyNWUxZjRmMTg1ICgiY2FuOiBwZWFrX3VzYjogYWRkIHN1
cHBvcnQgZm9yIFBFQUsgbmV3IENBTkZEIFVTQiBhZGFwdGVycyIpDQpTaWduZWQtb2ZmLWJ5OiBE
YW4gQ2FycGVudGVyIDxkYW4uY2FycGVudGVyQG9yYWNsZS5jb20+DQotLS0NCiBkcml2ZXJzL25l
dC9jYW4vdXNiL3BlYWtfdXNiL3BjYW5fdXNiX2ZkLmMgfCA0OCArKysrKysrKysrKysrKysrKy0t
LS0tDQogMSBmaWxlIGNoYW5nZWQsIDM3IGluc2VydGlvbnMoKyksIDExIGRlbGV0aW9ucygtKQ0K
DQpkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvY2FuL3VzYi9wZWFrX3VzYi9wY2FuX3VzYl9mZC5j
IGIvZHJpdmVycy9uZXQvY2FuL3VzYi9wZWFrX3VzYi9wY2FuX3VzYl9mZC5jDQppbmRleCA0N2Nj
MWZmNWI4OGUuLmRlZTNlNjg5YjU0ZCAxMDA2NDQNCi0tLSBhL2RyaXZlcnMvbmV0L2Nhbi91c2Iv
cGVha191c2IvcGNhbl91c2JfZmQuYw0KKysrIGIvZHJpdmVycy9uZXQvY2FuL3VzYi9wZWFrX3Vz
Yi9wY2FuX3VzYl9mZC5jDQpAQCAtNDY4LDEyICs0NjgsMTggQEAgc3RhdGljIGludCBwY2FuX3Vz
Yl9mZF9kZWNvZGVfY2FubXNnKHN0cnVjdCBwY2FuX3VzYl9mZF9pZiAqdXNiX2lmLA0KICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgcHVjYW5fbXNnICpyeF9tc2cp
DQogew0KICAgICAgICAgc3RydWN0IHB1Y2FuX3J4X21zZyAqcm0gPSAoc3RydWN0IHB1Y2FuX3J4
X21zZyAqKXJ4X21zZzsNCi0gICAgICAgc3RydWN0IHBlYWtfdXNiX2RldmljZSAqZGV2ID0gdXNi
X2lmLT5kZXZbcHVjYW5fbXNnX2dldF9jaGFubmVsKHJtKV07DQotICAgICAgIHN0cnVjdCBuZXRf
ZGV2aWNlICpuZXRkZXYgPSBkZXYtPm5ldGRldjsNCisgICAgICAgc3RydWN0IHBlYWtfdXNiX2Rl
dmljZSAqZGV2Ow0KKyAgICAgICBzdHJ1Y3QgbmV0X2RldmljZSAqbmV0ZGV2Ow0KICAgICAgICAg
c3RydWN0IGNhbmZkX2ZyYW1lICpjZmQ7DQogICAgICAgICBzdHJ1Y3Qgc2tfYnVmZiAqc2tiOw0K
ICAgICAgICAgY29uc3QgdTE2IHJ4X21zZ19mbGFncyA9IGxlMTZfdG9fY3B1KHJtLT5mbGFncyk7
DQoNCisgICAgICAgaWYgKHB1Y2FuX21zZ19nZXRfY2hhbm5lbChybSkgPj0gQVJSQVlfU0laRSh1
c2JfaWYtPmRldikpDQorICAgICAgICAgICAgICAgcmV0dXJuIC1FTk9NRU07DQorDQorICAgICAg
IGRldiA9IHVzYl9pZi0+ZGV2W3B1Y2FuX21zZ19nZXRfY2hhbm5lbChybSldOw0KKyAgICAgICBu
ZXRkZXYgPSBkZXYtPm5ldGRldjsNCisNCiAgICAgICAgIGlmIChyeF9tc2dfZmxhZ3MgJiBQVUNB
Tl9NU0dfRVhUX0RBVEFfTEVOKSB7DQogICAgICAgICAgICAgICAgIC8qIENBTkZEIGZyYW1lIGNh
c2UgKi8NCiAgICAgICAgICAgICAgICAgc2tiID0gYWxsb2NfY2FuZmRfc2tiKG5ldGRldiwgJmNm
ZCk7DQpAQCAtNTE5LDE1ICs1MjUsMjEgQEAgc3RhdGljIGludCBwY2FuX3VzYl9mZF9kZWNvZGVf
c3RhdHVzKHN0cnVjdCBwY2FuX3VzYl9mZF9pZiAqdXNiX2lmLA0KICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBzdHJ1Y3QgcHVjYW5fbXNnICpyeF9tc2cpDQogew0KICAgICAg
ICAgc3RydWN0IHB1Y2FuX3N0YXR1c19tc2cgKnNtID0gKHN0cnVjdCBwdWNhbl9zdGF0dXNfbXNn
ICopcnhfbXNnOw0KLSAgICAgICBzdHJ1Y3QgcGVha191c2JfZGV2aWNlICpkZXYgPSB1c2JfaWYt
PmRldltwdWNhbl9zdG1zZ19nZXRfY2hhbm5lbChzbSldOw0KLSAgICAgICBzdHJ1Y3QgcGNhbl91
c2JfZmRfZGV2aWNlICpwZGV2ID0NCi0gICAgICAgICAgICAgICAgICAgICAgIGNvbnRhaW5lcl9v
ZihkZXYsIHN0cnVjdCBwY2FuX3VzYl9mZF9kZXZpY2UsIGRldik7DQorICAgICAgIHN0cnVjdCBw
Y2FuX3VzYl9mZF9kZXZpY2UgKnBkZXY7DQogICAgICAgICBlbnVtIGNhbl9zdGF0ZSBuZXdfc3Rh
dGUgPSBDQU5fU1RBVEVfRVJST1JfQUNUSVZFOw0KICAgICAgICAgZW51bSBjYW5fc3RhdGUgcnhf
c3RhdGUsIHR4X3N0YXRlOw0KLSAgICAgICBzdHJ1Y3QgbmV0X2RldmljZSAqbmV0ZGV2ID0gZGV2
LT5uZXRkZXY7DQorICAgICAgIHN0cnVjdCBwZWFrX3VzYl9kZXZpY2UgKmRldjsNCisgICAgICAg
c3RydWN0IG5ldF9kZXZpY2UgKm5ldGRldjsNCiAgICAgICAgIHN0cnVjdCBjYW5fZnJhbWUgKmNm
Ow0KICAgICAgICAgc3RydWN0IHNrX2J1ZmYgKnNrYjsNCg0KKyAgICAgICBpZiAocHVjYW5fc3Rt
c2dfZ2V0X2NoYW5uZWwoc20pID49IEFSUkFZX1NJWkUodXNiX2lmLT5kZXYpKQ0KKyAgICAgICAg
ICAgICAgIHJldHVybiAtRU5PTUVNOw0KKw0KKyAgICAgICBkZXYgPSB1c2JfaWYtPmRldltwdWNh
bl9zdG1zZ19nZXRfY2hhbm5lbChzbSldOw0KKyAgICAgICBwZGV2ID0gY29udGFpbmVyX29mKGRl
diwgc3RydWN0IHBjYW5fdXNiX2ZkX2RldmljZSwgZGV2KTsNCisgICAgICAgbmV0ZGV2ID0gZGV2
LT5uZXRkZXY7DQorDQogICAgICAgICAvKiBub3RoaW5nIHNob3VsZCBiZSBzZW50IHdoaWxlIGlu
IEJVU19PRkYgc3RhdGUgKi8NCiAgICAgICAgIGlmIChkZXYtPmNhbi5zdGF0ZSA9PSBDQU5fU1RB
VEVfQlVTX09GRikNCiAgICAgICAgICAgICAgICAgcmV0dXJuIDA7DQpAQCAtNTc5LDkgKzU5MSwx
NCBAQCBzdGF0aWMgaW50IHBjYW5fdXNiX2ZkX2RlY29kZV9lcnJvcihzdHJ1Y3QgcGNhbl91c2Jf
ZmRfaWYgKnVzYl9pZiwNCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzdHJ1
Y3QgcHVjYW5fbXNnICpyeF9tc2cpDQogew0KICAgICAgICAgc3RydWN0IHB1Y2FuX2Vycm9yX21z
ZyAqZXIgPSAoc3RydWN0IHB1Y2FuX2Vycm9yX21zZyAqKXJ4X21zZzsNCi0gICAgICAgc3RydWN0
IHBlYWtfdXNiX2RldmljZSAqZGV2ID0gdXNiX2lmLT5kZXZbcHVjYW5fZXJtc2dfZ2V0X2NoYW5u
ZWwoZXIpXTsNCi0gICAgICAgc3RydWN0IHBjYW5fdXNiX2ZkX2RldmljZSAqcGRldiA9DQotICAg
ICAgICAgICAgICAgICAgICAgICBjb250YWluZXJfb2YoZGV2LCBzdHJ1Y3QgcGNhbl91c2JfZmRf
ZGV2aWNlLCBkZXYpOw0KKyAgICAgICBzdHJ1Y3QgcGNhbl91c2JfZmRfZGV2aWNlICpwZGV2Ow0K
KyAgICAgICBzdHJ1Y3QgcGVha191c2JfZGV2aWNlICpkZXY7DQorDQorICAgICAgIGlmIChwdWNh
bl9lcm1zZ19nZXRfY2hhbm5lbChlcikgPj0gQVJSQVlfU0laRSh1c2JfaWYtPmRldikpDQorICAg
ICAgICAgICAgICAgcmV0dXJuIC1FSU5WQUw7DQorDQorICAgICAgIGRldiA9IHVzYl9pZi0+ZGV2
W3B1Y2FuX2VybXNnX2dldF9jaGFubmVsKGVyKV07DQorICAgICAgIHBkZXYgPSBjb250YWluZXJf
b2YoZGV2LCBzdHJ1Y3QgcGNhbl91c2JfZmRfZGV2aWNlLCBkZXYpOw0KDQogICAgICAgICAvKiBr
ZWVwIGEgdHJhY2Ugb2YgdHggYW5kIHJ4IGVycm9yIGNvdW50ZXJzIGZvciBsYXRlciB1c2UgKi8N
CiAgICAgICAgIHBkZXYtPmJlYy50eGVyciA9IGVyLT50eF9lcnJfY250Ow0KQEAgLTU5NSwxMSAr
NjEyLDE3IEBAIHN0YXRpYyBpbnQgcGNhbl91c2JfZmRfZGVjb2RlX292ZXJydW4oc3RydWN0IHBj
YW5fdXNiX2ZkX2lmICp1c2JfaWYsDQogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICBzdHJ1Y3QgcHVjYW5fbXNnICpyeF9tc2cpDQogew0KICAgICAgICAgc3RydWN0IHBjYW5f
dWZkX292cl9tc2cgKm92ID0gKHN0cnVjdCBwY2FuX3VmZF9vdnJfbXNnICopcnhfbXNnOw0KLSAg
ICAgICBzdHJ1Y3QgcGVha191c2JfZGV2aWNlICpkZXYgPSB1c2JfaWYtPmRldltwdWZkX29tc2df
Z2V0X2NoYW5uZWwob3YpXTsNCi0gICAgICAgc3RydWN0IG5ldF9kZXZpY2UgKm5ldGRldiA9IGRl
di0+bmV0ZGV2Ow0KKyAgICAgICBzdHJ1Y3QgcGVha191c2JfZGV2aWNlICpkZXY7DQorICAgICAg
IHN0cnVjdCBuZXRfZGV2aWNlICpuZXRkZXY7DQogICAgICAgICBzdHJ1Y3QgY2FuX2ZyYW1lICpj
ZjsNCiAgICAgICAgIHN0cnVjdCBza19idWZmICpza2I7DQoNCisgICAgICAgaWYgKHB1ZmRfb21z
Z19nZXRfY2hhbm5lbChvdikgPj0gQVJSQVlfU0laRSh1c2JfaWYtPmRldikpDQorICAgICAgICAg
ICAgICAgcmV0dXJuIC1FSU5WQUw7DQorDQorICAgICAgIGRldiA9IHVzYl9pZi0+ZGV2W3B1ZmRf
b21zZ19nZXRfY2hhbm5lbChvdildOw0KKyAgICAgICBuZXRkZXYgPSBkZXYtPm5ldGRldjsNCisN
CiAgICAgICAgIC8qIGFsbG9jYXRlIGFuIHNrYiB0byBzdG9yZSB0aGUgZXJyb3IgZnJhbWUgKi8N
CiAgICAgICAgIHNrYiA9IGFsbG9jX2Nhbl9lcnJfc2tiKG5ldGRldiwgJmNmKTsNCiAgICAgICAg
IGlmICghc2tiKQ0KQEAgLTcxNiw2ICs3MzksOSBAQCBzdGF0aWMgaW50IHBjYW5fdXNiX2ZkX2Vu
Y29kZV9tc2coc3RydWN0IHBlYWtfdXNiX2RldmljZSAqZGV2LA0KICAgICAgICAgdTE2IHR4X21z
Z19zaXplLCB0eF9tc2dfZmxhZ3M7DQogICAgICAgICB1OCBjYW5fZGxjOw0KDQorICAgICAgIGlm
IChjZmQtPmxlbiA+IENBTkZEX01BWF9ETEVOKQ0KKyAgICAgICAgICAgICAgIHJldHVybiAtRUlO
VkFMOw0KKw0KICAgICAgICAgdHhfbXNnX3NpemUgPSBBTElHTihzaXplb2Yoc3RydWN0IHB1Y2Fu
X3R4X21zZykgKyBjZmQtPmxlbiwgNCk7DQogICAgICAgICB0eF9tc2ctPnNpemUgPSBjcHVfdG9f
bGUxNih0eF9tc2dfc2l6ZSk7DQogICAgICAgICB0eF9tc2ctPnR5cGUgPSBjcHVfdG9fbGUxNihQ
VUNBTl9NU0dfQ0FOX1RYKTsNCi0tDQoyLjI4LjANCg0KLS0NClBFQUstU3lzdGVtIFRlY2huaWsg
R21iSA0KU2l0eiBkZXIgR2VzZWxsc2NoYWZ0IERhcm1zdGFkdCAtIEhSQiA5MTgzDQpHZXNjaGFl
ZnRzZnVlaHJ1bmc6IEFsZXhhbmRlciBHYWNoIC8gVXdlIFdpbGhlbG0NClVuc2VyZSBEYXRlbnNj
aHV0emVya2xhZXJ1bmcgbWl0IHdpY2h0aWdlbiBIaW53ZWlzZW4NCnp1ciBCZWhhbmRsdW5nIHBl
cnNvbmVuYmV6b2dlbmVyIERhdGVuIGZpbmRlbiBTaWUgdW50ZXINCnd3dy5wZWFrLXN5c3RlbS5j
b20vRGF0ZW5zY2h1dHouNDgzLjAuaHRtbA0K
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] can: peak_usb: add range checking in decode operations
2020-10-26 13:28 ` Stéphane Grosjean
@ 2020-10-26 13:41 ` Marc Kleine-Budde
-1 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2020-10-26 13:41 UTC (permalink / raw)
To: Stéphane Grosjean, Dan Carpenter
Cc: David S. Miller, Jakub Kicinski, Andri Yngvason, Oliver Hartkopp,
linux-can, netdev, kernel-janitors, Wolfgang Grandegger
[-- Attachment #1.1: Type: text/plain, Size: 476 bytes --]
On 10/26/20 2:28 PM, Stéphane Grosjean wrote:
> Hello Dan,
>
> You have my Ack.
>
> Acked-by: Stephane Grosjean <s.grosjean@peak-system.com>
Applied to linux-can/testing.
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] can: peak_usb: add range checking in decode operations
@ 2020-10-26 13:41 ` Marc Kleine-Budde
0 siblings, 0 replies; 11+ messages in thread
From: Marc Kleine-Budde @ 2020-10-26 13:41 UTC (permalink / raw)
To: Stéphane Grosjean, Dan Carpenter
Cc: David S. Miller, Jakub Kicinski, Andri Yngvason, Oliver Hartkopp,
linux-can, netdev, kernel-janitors, Wolfgang Grandegger
[-- Attachment #1.1: Type: text/plain, Size: 476 bytes --]
On 10/26/20 2:28 PM, Stéphane Grosjean wrote:
> Hello Dan,
>
> You have my Ack.
>
> Acked-by: Stephane Grosjean <s.grosjean@peak-system.com>
Applied to linux-can/testing.
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-10-26 13:41 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-13 14:06 [PATCH net] can: peak_usb: add range checking in decode operations Dan Carpenter
2020-08-13 14:06 ` Dan Carpenter
2020-10-14 13:22 ` Stéphane Grosjean
2020-10-14 17:48 ` Oliver Hartkopp
2020-10-14 17:48 ` Oliver Hartkopp
[not found] ` <VI1PR03MB5053CAE3ED35D8E9C23063F8D6030@VI1PR03MB5053.eurprd03.prod.outlook.com>
2020-10-16 7:51 ` Marc Kleine-Budde
2020-10-16 7:51 ` Marc Kleine-Budde
2020-10-26 13:28 ` Stéphane Grosjean
2020-10-26 13:28 ` Stéphane Grosjean
2020-10-26 13:41 ` Marc Kleine-Budde
2020-10-26 13:41 ` 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.