diff mbox

mscan: too much data copied to CAN frame due to 16 bit accesses

Message ID 4E8F52CE.1000204@grandegger.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Wolfgang Grandegger Oct. 7, 2011, 7:28 p.m. UTC 
Due to the 16 bit access to mscan registers there's too much data copied to
the zero initialized CAN frame when having an odd number of bytes to copy.
This patch ensures that only the requested bytes are copied by using an
8 bit access for the remaining byte.

Reported-by: Andre Naujoks <nautsch@gmail.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/net/can/mscan/mscan.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

Comments

David Miller Oct. 10, 2011, 6:31 p.m. UTC | #1 
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Fri, 07 Oct 2011 21:28:14 +0200

> Due to the 16 bit access to mscan registers there's too much data copied to
> the zero initialized CAN frame when having an odd number of bytes to copy.
> This patch ensures that only the requested bytes are copied by using an
> 8 bit access for the remaining byte.
> 
> Reported-by: Andre Naujoks <nautsch@gmail.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>

Applied, thanks everyone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index ac42f5d..ec4a311 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -261,11 +261,13 @@  static netdev_tx_t mscan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		void __iomem *data = &regs->tx.dsr1_0;
 		u16 *payload = (u16 *)frame->data;
 
-		/* It is safe to write into dsr[dlc+1] */
-		for (i = 0; i < (frame->can_dlc + 1) / 2; i++) {
+		for (i = 0; i < frame->can_dlc / 2; i++) {
 			out_be16(data, *payload++);
 			data += 2 + _MSCAN_RESERVED_DSR_SIZE;
 		}
+		/* write remaining byte if necessary */
+		if (frame->can_dlc & 1)
+			out_8(data, frame->data[frame->can_dlc - 1]);
 	}
 
 	out_8(&regs->tx.dlr, frame->can_dlc);
@@ -330,10 +332,13 @@  static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame)
 		void __iomem *data = &regs->rx.dsr1_0;
 		u16 *payload = (u16 *)frame->data;
 
-		for (i = 0; i < (frame->can_dlc + 1) / 2; i++) {
+		for (i = 0; i < frame->can_dlc / 2; i++) {
 			*payload++ = in_be16(data);
 			data += 2 + _MSCAN_RESERVED_DSR_SIZE;
 		}
+		/* read remaining byte if necessary */
+		if (frame->can_dlc & 1)
+			frame->data[frame->can_dlc - 1] = in_8(data);
 	}
 
 	out_8(&regs->canrflg, MSCAN_RXF);