netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] can: bcm: fix support for CAN FD frames
@ 2016-11-23 13:33 Oliver Hartkopp
  2016-11-23 14:15 ` Andrey Konovalov
  2016-11-23 14:35 ` Marc Kleine-Budde
  0 siblings, 2 replies; 6+ messages in thread
From: Oliver Hartkopp @ 2016-11-23 13:33 UTC (permalink / raw)
  To: linux-can, Andrey Konovalov, netdev; +Cc: Oliver Hartkopp

Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
CAN broadcast manager supports CAN and CAN FD data frames.

As these data frames are embedded in struct can[fd]_frames which have a
different length the access to the provided array of CAN frames became
dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
CAN frames the new offset calculation based on op->cfsiz was accidently applied
to CAN FD frame element lengths.

This fix makes the pointer to the arrays of the different CAN frame types a
void pointer so that the offset calculation in bytes accesses the correct CAN
frame elements.

Reference: http://marc.info/?l=linux-netdev&m=147980658909653

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/bcm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8af9d25..436a753 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -77,7 +77,7 @@
 		     (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
 		     (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
 
-#define CAN_BCM_VERSION "20160617"
+#define CAN_BCM_VERSION "20161123"
 
 MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
 MODULE_LICENSE("Dual BSD/GPL");
@@ -109,8 +109,9 @@ struct bcm_op {
 	u32 count;
 	u32 nframes;
 	u32 currframe;
-	struct canfd_frame *frames;
-	struct canfd_frame *last_frames;
+	/* void pointers to arrays of struct can[fd]_frame */
+	void *frames;
+	void *last_frames;
 	struct canfd_frame sframe;
 	struct canfd_frame last_sframe;
 	struct sock *sk;
@@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
 
 	if (op->flags & RX_FILTER_ID) {
 		/* the easiest case */
-		bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
+		bcm_rx_update_and_send(op, op->last_frames, rxframe);
 		goto rx_starttimer;
 	}
 
@@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 
 		if (msg_head->nframes) {
 			/* update CAN frames content */
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0)
 				return err;
@@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		}
 
 		if (msg_head->nframes) {
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0) {
 				if (op->frames != &op->sframe)
@@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 	/* check flags */
 
 	if (op->flags & RX_RTR_FRAME) {
+		struct canfd_frame *frame0 = op->frames;
 
 		/* no timers in RTR-mode */
 		hrtimer_cancel(&op->thrtimer);
@@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		 * prevent a full-load-loopback-test ... ;-]
 		 */
 		if ((op->flags & TX_CP_CAN_ID) ||
-		    (op->frames[0].can_id == op->can_id))
-			op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
+		    (frame0->can_id == op->can_id))
+			frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
 
 	} else {
 		if (op->flags & SETTIMER) {
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] can: bcm: fix support for CAN FD frames
  2016-11-23 13:33 [PATCH] can: bcm: fix support for CAN FD frames Oliver Hartkopp
@ 2016-11-23 14:15 ` Andrey Konovalov
  2016-11-23 14:35 ` Marc Kleine-Budde
  1 sibling, 0 replies; 6+ messages in thread
From: Andrey Konovalov @ 2016-11-23 14:15 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can, netdev

On Wed, Nov 23, 2016 at 2:33 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
> CAN broadcast manager supports CAN and CAN FD data frames.
>
> As these data frames are embedded in struct can[fd]_frames which have a
> different length the access to the provided array of CAN frames became
> dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
> CAN frames the new offset calculation based on op->cfsiz was accidently applied
> to CAN FD frame element lengths.
>
> This fix makes the pointer to the arrays of the different CAN frame types a
> void pointer so that the offset calculation in bytes accesses the correct CAN
> frame elements.
>
> Reference: http://marc.info/?l=linux-netdev&m=147980658909653
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
>  net/can/bcm.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 8af9d25..436a753 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -77,7 +77,7 @@
>                      (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
>                      (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
>
> -#define CAN_BCM_VERSION "20160617"
> +#define CAN_BCM_VERSION "20161123"
>
>  MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
>  MODULE_LICENSE("Dual BSD/GPL");
> @@ -109,8 +109,9 @@ struct bcm_op {
>         u32 count;
>         u32 nframes;
>         u32 currframe;
> -       struct canfd_frame *frames;
> -       struct canfd_frame *last_frames;
> +       /* void pointers to arrays of struct can[fd]_frame */
> +       void *frames;
> +       void *last_frames;
>         struct canfd_frame sframe;
>         struct canfd_frame last_sframe;
>         struct sock *sk;
> @@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
>
>         if (op->flags & RX_FILTER_ID) {
>                 /* the easiest case */
> -               bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
> +               bcm_rx_update_and_send(op, op->last_frames, rxframe);
>                 goto rx_starttimer;
>         }
>
> @@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>
>                 if (msg_head->nframes) {
>                         /* update CAN frames content */
> -                       err = memcpy_from_msg((u8 *)op->frames, msg,
> +                       err = memcpy_from_msg(op->frames, msg,
>                                               msg_head->nframes * op->cfsiz);
>                         if (err < 0)
>                                 return err;
> @@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>                 }
>
>                 if (msg_head->nframes) {
> -                       err = memcpy_from_msg((u8 *)op->frames, msg,
> +                       err = memcpy_from_msg(op->frames, msg,
>                                               msg_head->nframes * op->cfsiz);
>                         if (err < 0) {
>                                 if (op->frames != &op->sframe)
> @@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>         /* check flags */
>
>         if (op->flags & RX_RTR_FRAME) {
> +               struct canfd_frame *frame0 = op->frames;
>
>                 /* no timers in RTR-mode */
>                 hrtimer_cancel(&op->thrtimer);
> @@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>                  * prevent a full-load-loopback-test ... ;-]
>                  */
>                 if ((op->flags & TX_CP_CAN_ID) ||
> -                   (op->frames[0].can_id == op->can_id))
> -                       op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
> +                   (frame0->can_id == op->can_id))
> +                       frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
>
>         } else {
>                 if (op->flags & SETTIMER) {
> --
> 2.10.2
>

Tested-by: Andrey Konovalov <andreyknvl@google.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* pull-request: can 2016-11-23
@ 2016-11-23 14:34 Marc Kleine-Budde
  2016-11-23 14:34 ` [PATCH] can: bcm: fix support for CAN FD frames Marc Kleine-Budde
  2016-11-25 21:18 ` pull-request: can 2016-11-23 David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2016-11-23 14:34 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

this is a pull request for net/master.

The patch by Oliver Hartkopp for the broadcast manager (bcm) fixes the CAN-FD
support, which may cause an out-of-bounds access otherwise.

regards,
Marc
---

The following changes since commit c9b8af1330198ae241cd545e1f040019010d44d9:

  flow_dissect: call init_default_flow_dissectors() earlier (2016-11-22 14:44:01 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.9-20161123

for you to fetch changes up to 5499a6b22e5508b921c447757685b0a5e40a07ed:

  can: bcm: fix support for CAN FD frames (2016-11-23 15:22:18 +0100)

----------------------------------------------------------------
linux-can-fixes-for-4.9-20161123

----------------------------------------------------------------
Oliver Hartkopp (1):
      can: bcm: fix support for CAN FD frames

 net/can/bcm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] can: bcm: fix support for CAN FD frames
  2016-11-23 14:34 pull-request: can 2016-11-23 Marc Kleine-Budde
@ 2016-11-23 14:34 ` Marc Kleine-Budde
  2016-11-25 21:18 ` pull-request: can 2016-11-23 David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2016-11-23 14:34 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Oliver Hartkopp, linux-stable,
	Marc Kleine-Budde

From: Oliver Hartkopp <socketcan@hartkopp.net>

Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
CAN broadcast manager supports CAN and CAN FD data frames.

As these data frames are embedded in struct can[fd]_frames which have a
different length the access to the provided array of CAN frames became
dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
CAN frames the new offset calculation based on op->cfsiz was accidently applied
to CAN FD frame element lengths.

This fix makes the pointer to the arrays of the different CAN frame types a
void pointer so that the offset calculation in bytes accesses the correct CAN
frame elements.

Reference: http://marc.info/?l=linux-netdev&m=147980658909653

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/bcm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8af9d25ff988..436a7537e6a9 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -77,7 +77,7 @@
 		     (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
 		     (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
 
-#define CAN_BCM_VERSION "20160617"
+#define CAN_BCM_VERSION "20161123"
 
 MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
 MODULE_LICENSE("Dual BSD/GPL");
@@ -109,8 +109,9 @@ struct bcm_op {
 	u32 count;
 	u32 nframes;
 	u32 currframe;
-	struct canfd_frame *frames;
-	struct canfd_frame *last_frames;
+	/* void pointers to arrays of struct can[fd]_frame */
+	void *frames;
+	void *last_frames;
 	struct canfd_frame sframe;
 	struct canfd_frame last_sframe;
 	struct sock *sk;
@@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
 
 	if (op->flags & RX_FILTER_ID) {
 		/* the easiest case */
-		bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
+		bcm_rx_update_and_send(op, op->last_frames, rxframe);
 		goto rx_starttimer;
 	}
 
@@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 
 		if (msg_head->nframes) {
 			/* update CAN frames content */
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0)
 				return err;
@@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		}
 
 		if (msg_head->nframes) {
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0) {
 				if (op->frames != &op->sframe)
@@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 	/* check flags */
 
 	if (op->flags & RX_RTR_FRAME) {
+		struct canfd_frame *frame0 = op->frames;
 
 		/* no timers in RTR-mode */
 		hrtimer_cancel(&op->thrtimer);
@@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		 * prevent a full-load-loopback-test ... ;-]
 		 */
 		if ((op->flags & TX_CP_CAN_ID) ||
-		    (op->frames[0].can_id == op->can_id))
-			op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
+		    (frame0->can_id == op->can_id))
+			frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
 
 	} else {
 		if (op->flags & SETTIMER) {
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] can: bcm: fix support for CAN FD frames
  2016-11-23 13:33 [PATCH] can: bcm: fix support for CAN FD frames Oliver Hartkopp
  2016-11-23 14:15 ` Andrey Konovalov
@ 2016-11-23 14:35 ` Marc Kleine-Budde
  1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2016-11-23 14:35 UTC (permalink / raw)
  To: Oliver Hartkopp, linux-can, Andrey Konovalov, netdev


[-- Attachment #1.1: Type: text/plain, Size: 1260 bytes --]

On 11/23/2016 02:33 PM, Oliver Hartkopp wrote:
> Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
> CAN broadcast manager supports CAN and CAN FD data frames.
> 
> As these data frames are embedded in struct can[fd]_frames which have a
> different length the access to the provided array of CAN frames became
> dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
> CAN frames the new offset calculation based on op->cfsiz was accidently applied
> to CAN FD frame element lengths.
> 
> This fix makes the pointer to the arrays of the different CAN frame types a
> void pointer so that the offset calculation in bytes accesses the correct CAN
> frame elements.
> 
> Reference: http://marc.info/?l=linux-netdev&m=147980658909653
> 
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

Added to can and send a pull request to David.

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: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: pull-request: can 2016-11-23
  2016-11-23 14:34 pull-request: can 2016-11-23 Marc Kleine-Budde
  2016-11-23 14:34 ` [PATCH] can: bcm: fix support for CAN FD frames Marc Kleine-Budde
@ 2016-11-25 21:18 ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2016-11-25 21:18 UTC (permalink / raw)
  To: mkl; +Cc: netdev, linux-can, kernel

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Wed, 23 Nov 2016 15:34:29 +0100

> this is a pull request for net/master.
> 
> The patch by Oliver Hartkopp for the broadcast manager (bcm) fixes the CAN-FD
> support, which may cause an out-of-bounds access otherwise.

Pulled, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-11-25 21:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-23 14:34 pull-request: can 2016-11-23 Marc Kleine-Budde
2016-11-23 14:34 ` [PATCH] can: bcm: fix support for CAN FD frames Marc Kleine-Budde
2016-11-25 21:18 ` pull-request: can 2016-11-23 David Miller
  -- strict thread matches above, loose matches on Subject: below --
2016-11-23 13:33 [PATCH] can: bcm: fix support for CAN FD frames Oliver Hartkopp
2016-11-23 14:15 ` Andrey Konovalov
2016-11-23 14:35 ` Marc Kleine-Budde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).