Commit c7cd606f authored by Oliver Hartkopp's avatar Oliver Hartkopp Committed by David S. Miller

can: Fix data length code handling in rx path

A valid CAN dataframe can have a data length code (DLC) of 0 .. 8 data bytes.

When reading the CAN controllers register the 4-bit value may contain values
from 0 .. 15 which may exceed the reserved space in the socket buffer!

The ISO 11898-1 Chapter 8.4.2.3 (DLC field) says that register values > 8
should be reduced to 8 without any error reporting or frame drop.

This patch introduces a new helper macro to cast a given 4-bit data length
code (dlc) to __u8 and ensure the DLC value to be max. 8 bytes.

The different handlings in the rx path of the CAN netdevice drivers are fixed.
Signed-off-by: default avatarOliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: default avatarWolfgang Grandegger <wg@grandegger.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent d90a909e
...@@ -474,7 +474,7 @@ static void at91_read_mb(struct net_device *dev, unsigned int mb, ...@@ -474,7 +474,7 @@ static void at91_read_mb(struct net_device *dev, unsigned int mb,
reg_msr = at91_read(priv, AT91_MSR(mb)); reg_msr = at91_read(priv, AT91_MSR(mb));
if (reg_msr & AT91_MSR_MRTR) if (reg_msr & AT91_MSR_MRTR)
cf->can_id |= CAN_RTR_FLAG; cf->can_id |= CAN_RTR_FLAG;
cf->can_dlc = min_t(__u8, (reg_msr >> 16) & 0xf, 8); cf->can_dlc = get_can_dlc((reg_msr >> 16) & 0xf);
*(u32 *)(cf->data + 0) = at91_read(priv, AT91_MDL(mb)); *(u32 *)(cf->data + 0) = at91_read(priv, AT91_MDL(mb));
*(u32 *)(cf->data + 4) = at91_read(priv, AT91_MDH(mb)); *(u32 *)(cf->data + 4) = at91_read(priv, AT91_MDH(mb));
......
...@@ -392,7 +392,7 @@ static void bfin_can_rx(struct net_device *dev, u16 isrc) ...@@ -392,7 +392,7 @@ static void bfin_can_rx(struct net_device *dev, u16 isrc)
cf->can_id |= CAN_RTR_FLAG; cf->can_id |= CAN_RTR_FLAG;
/* get data length code */ /* get data length code */
cf->can_dlc = bfin_read16(&reg->chl[obj].dlc); cf->can_dlc = get_can_dlc(bfin_read16(&reg->chl[obj].dlc) & 0xF);
/* get payload */ /* get payload */
for (i = 0; i < 8; i += 2) { for (i = 0; i < 8; i += 2) {
......
...@@ -403,9 +403,8 @@ static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf, ...@@ -403,9 +403,8 @@ static void mcp251x_hw_rx_frame(struct spi_device *spi, u8 *buf,
for (i = 1; i < RXBDAT_OFF; i++) for (i = 1; i < RXBDAT_OFF; i++)
buf[i] = mcp251x_read_reg(spi, RXBCTRL(buf_idx) + i); buf[i] = mcp251x_read_reg(spi, RXBCTRL(buf_idx) + i);
len = buf[RXBDLC_OFF] & RXBDLC_LEN_MASK;
if (len > 8) len = get_can_dlc(buf[RXBDLC_OFF] & RXBDLC_LEN_MASK);
len = 8;
for (; i < (RXBDAT_OFF + len); i++) for (; i < (RXBDAT_OFF + len); i++)
buf[i] = mcp251x_read_reg(spi, RXBCTRL(buf_idx) + i); buf[i] = mcp251x_read_reg(spi, RXBCTRL(buf_idx) + i);
} else { } else {
...@@ -455,13 +454,7 @@ static void mcp251x_hw_rx(struct spi_device *spi, int buf_idx) ...@@ -455,13 +454,7 @@ static void mcp251x_hw_rx(struct spi_device *spi, int buf_idx)
(buf[RXBSIDL_OFF] >> RXBSIDL_SHIFT); (buf[RXBSIDL_OFF] >> RXBSIDL_SHIFT);
} }
/* Data length */ /* Data length */
frame->can_dlc = buf[RXBDLC_OFF] & RXBDLC_LEN_MASK; frame->can_dlc = get_can_dlc(buf[RXBDLC_OFF] & RXBDLC_LEN_MASK);
if (frame->can_dlc > 8) {
dev_warn(&spi->dev, "invalid frame recevied\n");
priv->net->stats.rx_errors++;
dev_kfree_skb(skb);
return;
}
memcpy(frame->data, buf + RXBDAT_OFF, frame->can_dlc); memcpy(frame->data, buf + RXBDAT_OFF, frame->can_dlc);
priv->net->stats.rx_packets++; priv->net->stats.rx_packets++;
......
...@@ -297,7 +297,8 @@ static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame) ...@@ -297,7 +297,8 @@ static void mscan_get_rx_frame(struct net_device *dev, struct can_frame *frame)
frame->can_id |= can_id >> 1; frame->can_id |= can_id >> 1;
if (can_id & 1) if (can_id & 1)
frame->can_id |= CAN_RTR_FLAG; frame->can_id |= CAN_RTR_FLAG;
frame->can_dlc = in_8(&regs->rx.dlr) & 0xf;
frame->can_dlc = get_can_dlc(in_8(&regs->rx.dlr) & 0xf);
if (!(frame->can_id & CAN_RTR_FLAG)) { if (!(frame->can_id & CAN_RTR_FLAG)) {
void __iomem *data = &regs->rx.dsr1_0; void __iomem *data = &regs->rx.dsr1_0;
......
...@@ -293,15 +293,14 @@ static void sja1000_rx(struct net_device *dev) ...@@ -293,15 +293,14 @@ static void sja1000_rx(struct net_device *dev)
uint8_t fi; uint8_t fi;
uint8_t dreg; uint8_t dreg;
canid_t id; canid_t id;
uint8_t dlc;
int i; int i;
/* create zero'ed CAN frame buffer */
skb = alloc_can_skb(dev, &cf); skb = alloc_can_skb(dev, &cf);
if (skb == NULL) if (skb == NULL)
return; return;
fi = priv->read_reg(priv, REG_FI); fi = priv->read_reg(priv, REG_FI);
dlc = fi & 0x0F;
if (fi & FI_FF) { if (fi & FI_FF) {
/* extended frame format (EFF) */ /* extended frame format (EFF) */
...@@ -318,16 +317,15 @@ static void sja1000_rx(struct net_device *dev) ...@@ -318,16 +317,15 @@ static void sja1000_rx(struct net_device *dev)
| (priv->read_reg(priv, REG_ID2) >> 5); | (priv->read_reg(priv, REG_ID2) >> 5);
} }
if (fi & FI_RTR) if (fi & FI_RTR) {
id |= CAN_RTR_FLAG; id |= CAN_RTR_FLAG;
} else {
cf->can_id = id; cf->can_dlc = get_can_dlc(fi & 0x0F);
cf->can_dlc = dlc; for (i = 0; i < cf->can_dlc; i++)
for (i = 0; i < dlc; i++)
cf->data[i] = priv->read_reg(priv, dreg++); cf->data[i] = priv->read_reg(priv, dreg++);
}
while (i < 8) cf->can_id = id;
cf->data[i++] = 0;
/* release receive buffer */ /* release receive buffer */
priv->write_reg(priv, REG_CMR, CMD_RRB); priv->write_reg(priv, REG_CMR, CMD_RRB);
...@@ -335,7 +333,7 @@ static void sja1000_rx(struct net_device *dev) ...@@ -335,7 +333,7 @@ static void sja1000_rx(struct net_device *dev)
netif_rx(skb); netif_rx(skb);
stats->rx_packets++; stats->rx_packets++;
stats->rx_bytes += dlc; stats->rx_bytes += cf->can_dlc;
} }
static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status) static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
......
...@@ -552,7 +552,7 @@ static int ti_hecc_rx_pkt(struct ti_hecc_priv *priv, int mbxno) ...@@ -552,7 +552,7 @@ static int ti_hecc_rx_pkt(struct ti_hecc_priv *priv, int mbxno)
data = hecc_read_mbx(priv, mbxno, HECC_CANMCF); data = hecc_read_mbx(priv, mbxno, HECC_CANMCF);
if (data & HECC_CANMCF_RTR) if (data & HECC_CANMCF_RTR)
cf->can_id |= CAN_RTR_FLAG; cf->can_id |= CAN_RTR_FLAG;
cf->can_dlc = data & 0xF; cf->can_dlc = get_can_dlc(data & 0xF);
data = hecc_read_mbx(priv, mbxno, HECC_CANMDL); data = hecc_read_mbx(priv, mbxno, HECC_CANMDL);
*(u32 *)(cf->data) = cpu_to_be32(data); *(u32 *)(cf->data) = cpu_to_be32(data);
if (cf->can_dlc > 4) { if (cf->can_dlc > 4) {
......
...@@ -316,7 +316,7 @@ static void ems_usb_rx_can_msg(struct ems_usb *dev, struct ems_cpc_msg *msg) ...@@ -316,7 +316,7 @@ static void ems_usb_rx_can_msg(struct ems_usb *dev, struct ems_cpc_msg *msg)
return; return;
cf->can_id = le32_to_cpu(msg->msg.can_msg.id); cf->can_id = le32_to_cpu(msg->msg.can_msg.id);
cf->can_dlc = min_t(u8, msg->msg.can_msg.length, 8); cf->can_dlc = get_can_dlc(msg->msg.can_msg.length & 0xF);
if (msg->type == CPC_MSG_TYPE_EXT_CAN_FRAME || if (msg->type == CPC_MSG_TYPE_EXT_CAN_FRAME ||
msg->type == CPC_MSG_TYPE_EXT_RTR_FRAME) msg->type == CPC_MSG_TYPE_EXT_RTR_FRAME)
......
...@@ -51,6 +51,15 @@ struct can_priv { ...@@ -51,6 +51,15 @@ struct can_priv {
struct sk_buff **echo_skb; struct sk_buff **echo_skb;
}; };
/*
* get_can_dlc(value) - helper macro to cast a given data length code (dlc)
* to __u8 and ensure the dlc value to be max. 8 bytes.
*
* To be used in the CAN netdriver receive path to ensure conformance with
* ISO 11898-1 Chapter 8.4.2.3 (DLC field)
*/
#define get_can_dlc(i) (min_t(__u8, (i), 8))
struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max); struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max);
void free_candev(struct net_device *dev); void free_candev(struct net_device *dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment