Skip to contentSkip to conversation

Add Loongson CAN-FD controller driver

PATCH Superseded Binbin Zhou 2 participants 5 messages Sep 9 CAN network drivers

v8 of 9 Superseded Latest is v9 →

lore
Share & tools
lore
#0 lore raw

v8 cover Add Loongson CAN-FD controller driver

Hi all:

This patchset adds support for the CAN-FD controller found on Loongson CPUs.

Patch 1 introduces the basic CAN-FD controller driver with support for classic CAN and CAN FD, including bit timing, error handling, NAPI-based RX, and multiple TX buffers.

Patch 2 adds optional DMA support for RX path using the Loongson APB CMC DMA engine, which significantly reduces CPU load under high receive throughput.

The driver has been tested on Loongson-2K3000 platforms with various CAN/CAN FD traffic patterns.

Finally, I'd like to thank Bingxiong, the original author of this driver, for his efforts in working on the patch.

Thanks.
Binbin

=========
V8:
Patch (1/2):
- Use local copy of can_bittiming in set_btr to avoid corrupting persistent structure;
- Fix endianness and strict aliasing in TX/RX data paths;
- Fixed spelling errors;
Patch (2/2):
- Add overflow detection for RXDMA;
- Simplified callback function names

Link to V7:
https://lore.kernel.org/all/[email protected]/

V7:
Patch (1/2):
- Fix can_bittiming_const->tseg1_max;
- Fix TX buffer id in loongson_canfd_start_xmit();
- Add rwcnt checking in loongson_canfd_rx()
- Add netdev stats spinlock;
- Add net_ratelimit() to control log output in loongson_canfd_err_interrupt);
- Rewrite loongson_canfd_rx_napi() to clearly distinguish between the two scenarios: "no more data to process" and "quota exhausted".

Patch (2/2):
- Put dmaengine_terminate_sync() before loongson_canfd_rxdma_free();

Link to V6:
https://lore.kernel.org/all/[email protected]/

V6:
- Rebase on linux-can/master;
Patch (1/2):
- Drop CAN_CTRLMODE_3_SAMPLES ctrlmode;
- Replace kfree_skb() with dev_kfree_skb_any();
- Drop REG_CMD_ERCRST operation;
- Rewrite loongson_canfd_tx_interrupt() to prevent duplicate handling of TX completion';
Patch (2/2):
- Added a fallback to true when dmaengine_tx_status() returns an error;
- Replaced all direct regmap_read() from LOONGSON_CANFD_RX_DATA with the priv->get_rx_data() callback;
- Release the DMA channel and clear priv->rx_ch on dma_alloc_coherent() failure;

Link to V5:
https://lore.kernel.org/all/[email protected]/

V5:
Patch (1/2):
- Fix tseg1_max const value;
- Empty FIFO when SKB allocate failed.
- Use REG_FRAME_META1_RWCNT as the playload read count value;
- Only clean processed interrupt flags in loongson_canfd_err_interrupt();
- Refact loongson_canfd_tx_interrupt();
- Add LOONGSON_CANFD_TRV_DLY into loongson_canfd_volatile_reg();
Patch (2/2):
- Fix dma_request_chan() error path;
- Fix build error;
- Fix loongson_canfd_rxdma_init() error path;

Link to V4:
https://lore.kernel.org/all/[email protected]/

V4:
- Fix some issues from sashiko.
Patch (1/2):
- Update commit message;
- Use common APIs to calculate DLC;
- Correct CAN_RTR_FLAG judgement;
- Add skb check for CAN_STATE_ERROR_ACTIVE case;
- Use regmap_write() rather than regmap_update_bits() to disable all interrupts;
- Add napi_disable() in error paths;
- Add clock-frequency return value check;
- More comments for macros.

Link to V3:
https://lore.kernel.org/all/[email protected]/

V3:
Patch (1/2):
- Replace the switch-case statement with a calculated mask in loongson_canfd_get_txbuf_sts();
- Change loongson_canfd_reset() return type to void;
- Drop old code in loongson_canfd_set_btr();
- Rename loongson_canfd_set_mode() to loongson_canfd_set_conf_mode();
- Drop cf->__res* usage;
- Drop unnecessary macro definitions;
- Add netif_napi_del() in probe error route path;
- Replace ternary operator with if-else;
- Drop stats->rx_packets and stats->rx_bytes count in CAN error frames;
- Add ethtool_ops;
- Add CAN_CTRLMODE_CC_LEN8_DLC;

Patch (2/2):
- Fix W=2 warning;
- Rename get_rx_frc() -> get_rxbuf_empty();
- Put loongson_canfd_rxdma_remove() before free_candev();
- Adjust the RXDMA probe process.

Link to V2:
https://lore.kernel.org/all/[email protected]/

V2:
Patch (1/2):
- Put all code into one file;
- Add COMPILE_TEST Kconfig option;
- Rewrite Kconfig description;
- Use regmap_test_bits() to simplify bit field checks;
- Drop odd FIELD_GET() usage;
- Don't use FIELD_GET() for a single bit;
- Use an if/else instead of the ternary operator;
- Use the CAN TDC framework to get the SSP value;
- Use guard(spinlock)/scoped_guard(spinlock);
- Use netdev_debug() to be less verboss;
- Check for memory allocation failure;
- Add CAN_ERR_CNT flag;
- Drop unused REG_DATA_xx_yy_W_DATA_yy definition;
- Add more function comment;

Link to V1:
https://lore.kernel.org/all/[email protected]/

Binbin Zhou (2):
can: Add Loongson CAN-FD controller driver
can: loongson_canfd: Add RXDMA support

 MAINTAINERS                      |    7 +
 drivers/net/can/Kconfig          |   11 +
 drivers/net/can/Makefile         |    1 +
 drivers/net/can/loongson_canfd.c | 1829 ++++++++++++++++++++++++++++++
 4 files changed, 1848 insertions(+)
 create mode 100644 drivers/net/can/loongson_canfd.c

base-commit: dc4b95b8fee95113587e93ca116356032d271371

Signature · 1 line
2.52.0
#1 lore raw

PATCH v8 1/2 can: Add Loongson CAN-FD controller driver

Add a new driver for the Loongson CAN-FD controller, which is integrated into Loongson-2K series SoCs. The controller supports both CAN 2.0 and CAN-FD protocols, with up to 8 TX buffers and a flexible RX buffer.

The driver implements the core CAN device operations:
- NAPI-based receive path with interrupt masking to reduce overhead;
- TX echo support for accurate loopback and echo skb handling;
- Error reporting (warning, passive, bus-off) with detailed status;
- Support for common CAN controller modes: loopback, listen-only, one-shot, FD, and non-ISO FD;
- Auto TDC (Transmitter Delay Compensation) for FD data phase.

---
 MAINTAINERS                      |    7 +
 drivers/net/can/Kconfig          |   11 +
 drivers/net/can/Makefile         |    1 +
 drivers/net/can/loongson_canfd.c | 1623 ++++++++++++++++++++++++++++++
 4 files changed, 1642 insertions(+)
 create mode 100644 drivers/net/can/loongson_canfd.c
MAINTAINERS +7 -0
index 460cb7268845..676b2ff55391 100644
@@ -15219,6 +15219,13 @@ F:	Documentation/devicetree/bindings/sound/loongson,ls2k1000-i2s.yaml
 F:	sound/soc/loongson/loongson_*.c
 F:	sound/soc/loongson/loongson_*.h
 
+LOONGSON CAN FD DRIVER
+M:	Bingxiong Li <[email protected]>
+M:	Binbin Zhou <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/net/can/loongson_canfd/
+
 LOONGSON GPIO DRIVER
 M:	Yinbo Zhu <[email protected]>
 L:	[email protected]
drivers/net/can/Kconfig +11 -0
index a8fad6fe5302..95a0f898959c 100644
@@ -186,6 +186,17 @@ config CAN_KVASER_PCIEFD
 	    Kvaser M.2 PCIe 4xCAN
 	    Kvaser PCIe 8xCAN
 
+config CAN_LOONGSON_CANFD
+	tristate "Loongson CAN-FD controller"
+	depends on HAS_IOMEM
+	select REGMAP_MMIO
+	help
+	  This is a canfd driver switch for the Loongson platform,
+	  integrated with the Loongson-2K series SoCs.
+
+	  To compile as a module, choose M here: the module will be
+	  called loongson_canfd.
+
 config CAN_SLCAN
 	tristate "Serial / USB serial CAN Adaptors (slcan)"
 	depends on TTY
drivers/net/can/Makefile +1 -0
index 4010d17f8583..c4eae4595ce3 100644
@@ -27,6 +27,7 @@ obj-$(CONFIG_CAN_GRCAN)		+= grcan.o
 obj-$(CONFIG_CAN_IFI_CANFD)	+= ifi_canfd/
 obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
 obj-$(CONFIG_CAN_KVASER_PCIEFD)	+= kvaser_pciefd/
+obj-$(CONFIG_CAN_LOONGSON_CANFD)	+= loongson_canfd.o
 obj-$(CONFIG_CAN_MSCAN)		+= mscan/
 obj-$(CONFIG_CAN_M_CAN)		+= m_can/
 obj-$(CONFIG_CAN_PEAK_PCIEFD)	+= peak_canfd/
drivers/net/can/loongson_canfd.c new file +1623 -0

1,623 lines not on this page · raw message

Signature · 1 line
2.52.0
#2 lore raw

Re: [PATCH v8 1/2] can: Add Loongson CAN-FD controller driver

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider: - [High] Returning -EAGAIN on format error desynchronizes the RX FIFO and causes an infinite interrupt…

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] Returning -EAGAIN on format error desynchronizes the RX FIFO and causes an infinite interrupt storm.
- [High] Out-of-bounds write in the RX path when receiving Classical CAN frames with anomalous payload lengths.
- [High] TOCTOU races and incorrect clearing of Write-1-to-Clear (W1C) interrupt status bits cause permanent loss of interrupts.
- [Medium] Failure to use the correct DLC mapping for Classic CAN frames with DLC > 8.
- [Medium] Queue handling race in ndo_start_xmit causes false NETDEV_TX_BUSY returns and severe console spam.
- [Medium] Bit timing logic fails to redistribute oversized prop_seg, silently misconfiguring hardware timings.
- [Medium] Incorrect initial operational state set in loongson_canfd_chip_start.
- [Low] Unreachable dead code loop intended to drain the RX FIFO.
--

commit e517c4643a4070a6b8fcde3c6d3ac1988016c84b
Author: Binbin Zhou <[email protected]>

can: Add Loongson CAN-FD controller driver

This commit adds a new driver for the Loongson CAN-FD controller. It supports both CAN 2.0 and CAN-FD protocols, implements NAPI-based receive paths, TX echo support, error reporting, and various common CAN controller modes.

Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +0 -0
new file mode 100644
@@ -0,0 +1,1623 @@

[ ... ]

Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5 -0
+static void loongson_canfd_set_btr(struct net_device *ndev, struct can_bittiming *bt, bool nominal)
+{
+	struct loongson_canfd_priv *priv = netdev_priv(ndev);
+	u32 ph1_max = FIELD_MAX(REG_BTR_PH1);
+	u32 prop = bt->prop_seg, phase1 = bt->phase_seg1;

drivers/net/can/loongson_canfd.c line 571

Quoted 10 lines
drivers/net/can/loongson_canfd.c new file +10 -0
+	u32 btr = 0;
+
+	/* Hardware PROP is 7 bits, PH1 is 6 bits; redistribute if PH1 exceeds max */
+	if (phase1 > ph1_max) {
+		prop += phase1 - ph1_max;
+		phase1 = ph1_max;
+	}
+
+	if (nominal) {
+		btr = FIELD_PREP(REG_BTR_PROP, prop) |

[Severity: Medium]
Does this logic fail to redistribute the values if prop exceeds its 7-bit maximum?

If the user specifies prop_seg = 180 and phase_seg1 = 10, prop will exceed the maximum and its upper bits will be silently bitwise-truncated by the FIELD_PREP(REG_BTR_PROP, prop) macro, resulting in invalid hardware timings. Should it also shift the excess from prop to phase1?

[ ... ]

Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2 -0
+static int loongson_canfd_chip_start(struct net_device *ndev)
+{

[ ... ]

drivers/net/can/loongson_canfd.c line 761

Quoted 6 lines
drivers/net/can/loongson_canfd.c new file +6 -0
+	int_msk = FIELD_GET(REG_INT_MASK_SET, ~int_ena);
+
+	regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, int_msk);
+	regmap_write(priv->regmap, LOONGSON_CANFD_INT_ENA, int_ena);
+
+	priv->can.state = CAN_STATE_STOPPED;

[Severity: Medium]
Should the initial CAN state be set to CAN_STATE_ERROR_ACTIVE here instead of CAN_STATE_STOPPED?

The CAN subsystem conventions typically require transitioning the state to CAN_STATE_ERROR_ACTIVE when the hardware is enabled and traffic can flow.

Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5 -0
+
+	regmap_update_bits(priv->regmap, LOONGSON_CANFD_CONF, REG_CONF_ENA, REG_CONF_ENA);
+
+	return 0;
+}

[ ... ]

Quoted 3 lines
drivers/net/can/loongson_canfd.c new file +3 -0
+static bool loongson_canfd_insert_frame(struct loongson_canfd_priv *priv,
+					const struct canfd_frame *cf, u8 buf, bool isfdf)
+{

[ ... ]

drivers/net/can/loongson_canfd.c line 835

Quoted 7 lines
drivers/net/can/loongson_canfd.c new file +7 -0
+	if (isfdf) {
+		frame_hdr1 = REG_FRAME_META1_FDF;
+		if (cf->flags & CANFD_BRS)
+			frame_hdr1 |= REG_FRAME_META1_BRS;
+	}
+
+	frame_hdr1 |= FIELD_PREP(REG_FRAME_META1_DLC, can_fd_len2dlc(cf->len));

[Severity: Medium]
For Classic CAN frames requesting a DLC greater than 8, will can_fd_len2dlc(cf->len) result in an incorrect clamped DLC being transmitted?

The driver advertises CAN_CTRLMODE_CC_LEN8_DLC support, meaning a Classic CAN frame may have cf->len = 8 but a user-requested cf->len8_dlc > 8. Should can_get_cc_dlc() be used here for Classic CAN frames?

[ ... ]

Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2 -0
+static netdev_tx_t loongson_canfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{

[ ... ]

Quoted 4 lines
drivers/net/can/loongson_canfd.c new file +4 -0
+	for_each_clear_bit(i, &tx_brp, LOONGSON_CANFD_TXBUF_NUM) {
+		u32 transmission_complete_mask = 0x3 << (i * 2);
+
+		if (!(tx_bs & transmission_complete_mask)) {

drivers/net/can/loongson_canfd.c line 901

Quoted 10 lines
drivers/net/can/loongson_canfd.c new file +10 -0
+			buf_id = i;
+			break;
+		}
+	}
+
+	if (buf_id == LOONGSON_CANFD_TXBUF_NUM) {
+		netif_stop_queue(ndev);
+		netdev_err(ndev, "No writable TX buffer, stopping queue\n");
+		return NETDEV_TX_BUSY;
+	}

[Severity: Medium]
Could this condition trigger erroneously during a normal transient race window?

A buffer whose pending bit REG_TX_STAT_BRP is 0 might have its software status BS set to TX_BS_VALID if the hardware completed transmission but the ISR hasn't cleaned it yet. In that case, no buf_id will be found, resulting in console spam and a false queue stoppage.

[ ... ]

Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2 -0
+static int loongson_canfd_rx(struct net_device *ndev)
+{

[ ... ]

drivers/net/can/loongson_canfd.c line 947

Quoted 10 lines
drivers/net/can/loongson_canfd.c new file +10 -0
+	regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &frame_hdr0);
+	regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &frame_hdr1);
+
+	rwcnt = FIELD_GET(REG_FRAME_META1_RWCNT, frame_hdr1);
+	if (rwcnt < 2)
+		return -EAGAIN;
+
+	dbcnt = (rwcnt - 2) * LOONGSON_CANFD_DW_BYTE;
+	if (dbcnt > CANFD_MAX_DLEN)
+		return -EAGAIN;

[Severity: High]
By returning -EAGAIN early here without draining the remaining unread payload words from the hardware FIFO, will this permanently desynchronize the RX path?

It appears this would abort the NAPI poll while leaving data in the buffer. When NAPI completes, the RX Not Empty interrupt is unmasked and immediately fires again, causing an infinite interrupt storm.

Quoted 8 lines
drivers/net/can/loongson_canfd.c new file +8 -0
+
+	is_can_fd = !!(frame_hdr1 & REG_FRAME_META1_FDF);
+	if (is_can_fd) {
+		skb = alloc_canfd_skb(ndev, &cfd);
+	} else {
+		skb = alloc_can_skb(ndev, &ccf);
+		cfd = (struct canfd_frame *)ccf;
+	}

[ ... ]

drivers/net/can/loongson_canfd.c line 989

Quoted 4 lines
drivers/net/can/loongson_canfd.c new file +4 -0
+	for (i = 0; i < dbcnt; i += LOONGSON_CANFD_DW_BYTE) {
+		regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
+		put_unaligned_le32(data, cfd->data + i);
+	}

[Severity: High]
Does this loop allow out-of-bounds writes if a Classical CAN frame has an anomalous payload length?

If the hardware returns a frame where FDF=0 but with rwcnt up
to 18 (dbcnt = 64 bytes), the length check dbcnt > CANFD_MAX_DLEN allows it. The alloc_can_skb() function only provides an 8-byte data array for Classic CAN frames, so this loop would write up to 64 bytes, overflowing the buffer and corrupting the skb_shared_info structure.

drivers/net/can/loongson_canfd.c line 994

Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5 -0
+
+	while (unlikely(i < dbcnt)) {
+		regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
+		i += LOONGSON_CANFD_DW_BYTE;
+	}

[Severity: Low]
Is this loop unreachable dead code?

Since dbcnt is mathematically guaranteed to be a multiple of 4, the prior for loop exactly terminates when i == dbcnt, making the i < dbcnt condition strictly false here.

[ ... ]

Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2 -0
+static void loongson_canfd_err_interrupt(struct net_device *ndev, u32 isr)
+{

[ ... ]

drivers/net/can/loongson_canfd.c line 1187

Quoted 4 lines
drivers/net/can/loongson_canfd.c new file +4 -0
+	if (skb)
+		netif_rx(skb);
+
+	regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, mask);

[Severity: High]
Does double-clearing the Write-1-to-Clear (W1C) error interrupt bits here cause a TOCTOU race?

The main ISR loongson_canfd_interrupt() appears to have already cleared these bits, so writing to them again could wipe out newly arrived interrupts.

Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5 -0
+	regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, mask << 16);
+}
+
+static int loongson_canfd_rx_napi(struct napi_struct *napi, int quota)
+{

[ ... ]

drivers/net/can/loongson_canfd.c line 1245

Quoted 3 lines
drivers/net/can/loongson_canfd.c new file +3 -0
+	if (no_more_data) {
+		if (napi_complete_done(napi, work_done)) {
+			regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, REG_INT_STAT_RBNEI);

[Severity: High]
Is it safe to clear the REG_INT_STAT_RBNEI interrupt flag at the end of the NAPI poll?

This could create a race condition where any RX interrupts that arrived after the empty check are dropped.

Quoted 10 lines
drivers/net/can/loongson_canfd.c new file +10 -0
+			regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK,
+				     REG_INT_STAT_RBNEI << 16);
+		}
+	}
+
+	return work_done;
+}
+
+static void loongson_canfd_tx_interrupt(struct net_device *ndev)
+{

[ ... ]

drivers/net/can/loongson_canfd.c line 1299

Quoted 4 lines
drivers/net/can/loongson_canfd.c new file +4 -0
+		loongson_canfd_set_txbuf_cmd(ndev, TXT_CMD_SR_CLEAN, buf_id);
+	}
+
+	regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, REG_INT_STAT_TXBHCI);

[Severity: High]
Similar to the above, does clearing the aggregate TX completion interrupt flag after checking the queues create a race window where missed completions could stall the TX queue?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#3 lore raw

PATCH v8 2/2 can: loongson_canfd: Add RXDMA support

In reply to Binbin Zhou

Extend the driver with an optional DMA engine support for the receive path. When a DMA channel named rx is available (e.g., from the loongson2-apb-cmc-dma driver), the RX buffer is mapped to a cyclic DMA transfer that continuously fills a pre-allocated ring buffer. The driver then readds incoming CAN frames directly from the DMA buffer, reducing the number of MMIO accesses and improving performance under high traffic.

---
 drivers/net/can/loongson_canfd.c | 250 ++++++++++++++++++++++++++++---
 1 file changed, 228 insertions(+), 22 deletions(-)
drivers/net/can/loongson_canfd.c +228 -22
index 6c983987a120..7fb28bd00c14 100644
@@ -6,11 +6,15 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/acpi_dma.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
 #include <linux/cleanup.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-direction.h>
+#include <linux/dma-mapping.h>
 #include <linux/ethtool.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
@@ -341,8 +345,10 @@
 #define DEV_NAME			"loongson_canfd"
 #define LOONGSON_CANFD_ID		0xBABE
 #define LOONGSON_CANFD_DW_BYTE		4
+#define LOONGSON_CANFD_RXBUF_SZ		SZ_1K
 #define LOONGSON_CANFD_TXBUF_NUM	8
 #define LOONGSON_CANFD_MAX_RTXTH	0xf
+#define LOONGSON_CANFD_RXDMA_NUM	(LOONGSON_CANFD_RXBUF_SZ / DMA_SLAVE_BUSWIDTH_4_BYTES)
 
 /**
  * struct loongson_canfd_priv - Driver private data
@@ -352,6 +358,13 @@
  * @res: Pointer to the CAN device resource (MMIO region)
  * @tx_lock: Lock for synchronizing TX interrupt handling
  * @stats_lock: Lock for protecting netdev statistics
+ * @rx_ch: DMA receive channel
+ * @rx_cookie: DMA receive cookie
+ * @rx_dma_buf: DMA RX buffer bus address
+ * @rx_buf: DMA RX buffer CPU address
+ * @rx_rd_ptr_dist: Read pointer distance from buffer end (in 32-bit words)
+ * @get_rx_data: Callback for reading RX data (DMA or poll mode)
+ * @get_rxbuf_empty: Callback for checking RX buffer empty state
  */
 struct loongson_canfd_priv {
 	struct can_priv		can;
@@ -360,6 +373,13 @@ struct loongson_canfd_priv {
 	struct resource		*res;
 	spinlock_t		tx_lock;	/* protect the sending queue */
 	spinlock_t		stats_lock;	/* protect the ndev->stats */
+	struct dma_chan		*rx_ch;
+	dma_cookie_t		rx_cookie;
+	dma_addr_t		rx_dma_buf;
+	unsigned int		*rx_buf;
+	u16			rx_rd_ptr_dist;
+	u32 (*get_rx_data)(struct loongson_canfd_priv *priv);
+	bool (*get_rxbuf_empty)(struct loongson_canfd_priv *priv);
 };
 
 /**
@@ -523,12 +543,166 @@ static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev,
 }
 
 /**
- * loongson_canfd_rxbuf_empty() - Check if RX buffer is empty
+ * loongson_canfd_rxdata_dma() - Read RX data in DMA mode
+ * @priv: Driver private data
+ *
+ * Reads the next 32-bit word from the DMA RX buffer, advancing the read pointer.
+ *
+ * Return: The next 32-bit data word.
+ */
+static u32 loongson_canfd_rxdata_dma(struct loongson_canfd_priv *priv)
+{
+	u32 data = 0;
+
+	data = priv->rx_buf[LOONGSON_CANFD_RXDMA_NUM - priv->rx_rd_ptr_dist--];
+	if (!priv->rx_rd_ptr_dist)
+		priv->rx_rd_ptr_dist = LOONGSON_CANFD_RXDMA_NUM;
+
+	return data;
+}
+
+/**
+ * loongson_canfd_rxbuf_empty_dma() - Check if RX buffer is empty in DMA mode
+ * @priv: Driver private data
+ *
+ * Uses DMA transfer status to compare read and write pointers.
+ * If the write pointer has wrapped past the read pointer, data loss is assumed,
+ * and the read pointer is advanced to the write position.
+ *
+ * Return: true if buffer is empty, false if data is pending.
+ */
+static bool loongson_canfd_rxbuf_empty_dma(struct loongson_canfd_priv *priv)
+{
+	struct dma_tx_state state;
+	enum dma_status status;
+	u32 wptr_dist;
+
+	status = dmaengine_tx_status(priv->rx_ch, priv->rx_cookie, &state);
+	if (status != DMA_IN_PROGRESS)
+		return true;
+
+	/* Convert residue from bytes to number of 32-bit words */
+	wptr_dist = state.residue / DMA_SLAVE_BUSWIDTH_4_BYTES;
+
+	/*
+	 * wptr_dist is the write pointer's distance from the buffer end.
+	 * It starts at BUFFER_SIZE (when write pointer is at start) and
+	 * decreases to 0 (when write pointer reaches end), then wraps back.
+	 * priv->rx_rd_ptr_dist is the read pointer's distance from the end.
+	 * If write pointer wraps past read pointer, wptr_dist becomes larger,
+	 * indicating data loss, so we advance read pointer to write position.
+	 */
+	if (priv->rx_rd_ptr_dist < wptr_dist)
+		priv->rx_rd_ptr_dist = wptr_dist;
+
+	return priv->rx_rd_ptr_dist == wptr_dist;
+}
+
+static void loongson_canfd_rxdma_free(struct loongson_canfd_priv *priv, struct device *dev)
+{
+	if (!priv->rx_buf)
+		return;
+
+	dma_free_coherent(dev, LOONGSON_CANFD_RXBUF_SZ, priv->rx_buf, priv->rx_dma_buf);
+	priv->rx_buf = NULL;
+}
+
+static void loongson_canfd_rxdma_remove(struct loongson_canfd_priv *priv, struct device *dev)
+{
+	if (!priv->rx_ch)
+		return;
+
+	dmaengine_terminate_sync(priv->rx_ch);
+	loongson_canfd_rxdma_free(priv, dev);
+	dma_release_channel(priv->rx_ch);
+	priv->rx_ch = NULL;
+}
+
+/**
+ * loongson_canfd_rxdma_init() - Initialize RX DMA
+ * @ndev: Network device
+ *
+ * Allocates DMA buffer and configures cyclic DMA transfer.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+static int loongson_canfd_rxdma_init(struct net_device *ndev)
+{
+	struct loongson_canfd_priv *priv = netdev_priv(ndev);
+	struct dma_async_tx_descriptor *desc = NULL;
+	struct device *dev = ndev->dev.parent;
+	struct dma_slave_config config;
+	int ret;
+
+	if (!priv->rx_ch)
+		return -EINVAL;
+
+	priv->rx_buf = dma_alloc_coherent(dev, LOONGSON_CANFD_RXBUF_SZ,
+					  &priv->rx_dma_buf, GFP_KERNEL);
+	if (!priv->rx_buf) {
+		dma_release_channel(priv->rx_ch);
+		priv->rx_ch = NULL;
+		return -ENOMEM;
+	}
+
+	memset(&config, 0, sizeof(config));
+	config.src_addr = priv->res->start + LOONGSON_CANFD_RX_DATA;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+
+	ret = dmaengine_slave_config(priv->rx_ch, &config);
+	if (ret < 0) {
+		netdev_err(ndev, "RX DMA channel config failed\n");
+		goto err_config;
+	}
+
+	desc = dmaengine_prep_dma_cyclic(priv->rx_ch, priv->rx_dma_buf,
+					 LOONGSON_CANFD_RXBUF_SZ, LOONGSON_CANFD_RXBUF_SZ,
+					 DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		netdev_err(ndev, "RX DMA cyclic transaction preparation failed\n");
+		ret = -EBUSY;
+		goto err_config;
+	}
+
+	priv->rx_cookie = dmaengine_submit(desc);
+	ret = dma_submit_error(priv->rx_cookie);
+	if (ret) {
+		dmaengine_terminate_sync(priv->rx_ch);
+		goto err_config;
+	}
+
+	dma_async_issue_pending(priv->rx_ch);
+
+	return 0;
+
+err_config:
+	loongson_canfd_rxdma_free(priv, dev);
+	dma_release_channel(priv->rx_ch);
+	priv->rx_ch = NULL;
+	return ret;
+}
+
+/**
+ * loongson_canfd_rxdata_poll() - Read RX data in poll mode
+ * @priv: Driver private data
+ *
+ * Return: The next 32-bit data word from the RX data register.
+ */
+static u32 loongson_canfd_rxdata_poll(struct loongson_canfd_priv *priv)
+{
+	u32 data;
+
+	regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
+	return data;
+}
+
+/**
+ * loongson_canfd_rxbuf_empty_poll() - Check if RX buffer is empty in poll mode
  * @priv: Driver private data
  *
  * Return: true if buffer is empty, false otherwise.
  */
-static bool loongson_canfd_rxbuf_empty(struct loongson_canfd_priv *priv)
+static bool loongson_canfd_rxbuf_empty_poll(struct loongson_canfd_priv *priv)
 {
 	return !!regmap_test_bits(priv->regmap, LOONGSON_CANFD_RX_STAT, REG_RX_STAT_RXE);
 }
@@ -747,8 +921,11 @@ static int loongson_canfd_chip_start(struct net_device *ndev)
 	loongson_canfd_set_conf_mode(priv);
 
 	/* Configure interrupts */
-	int_ena = REG_INT_STAT_RBNEI | REG_INT_STAT_TXBHCI |
-		  REG_INT_STAT_EWLI | REG_INT_STAT_FCSI;
+	int_ena = REG_INT_STAT_TXBHCI | REG_INT_STAT_EWLI | REG_INT_STAT_FCSI;
+	if (priv->rx_ch)
+		int_ena |= REG_INT_STAT_DMADI;
+	else
+		int_ena |= REG_INT_STAT_RBNEI;
 
 	if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
 		int_ena |= REG_INT_STAT_ALI | REG_INT_STAT_BEI;
@@ -929,14 +1106,14 @@ static int loongson_canfd_rx(struct net_device *ndev)
 {
 	struct loongson_canfd_priv *priv = netdev_priv(ndev);
 	struct net_device_stats *stats = &ndev->stats;
-	u32 frame_hdr0, frame_hdr1, dlc, rwcnt, dbcnt, i, data;
+	u32 frame_hdr0, frame_hdr1, dlc, rwcnt, dbcnt, i;
 	struct canfd_frame *cfd;
 	struct can_frame *ccf;
 	struct sk_buff *skb;
 	bool is_can_fd;
 
-	regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &frame_hdr0);
-	regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &frame_hdr1);
+	frame_hdr0 = priv->get_rx_data(priv);
+	frame_hdr1 = priv->get_rx_data(priv);
 
 	rwcnt = FIELD_GET(REG_FRAME_META1_RWCNT, frame_hdr1);
 	if (rwcnt < 2)
@@ -956,7 +1133,7 @@ static int loongson_canfd_rx(struct net_device *ndev)
 
 	if (unlikely(!skb)) {
 		for (i = 0; i < dbcnt; i += LOONGSON_CANFD_DW_BYTE)
-			regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
+			priv->get_rx_data(priv);
 		stats->rx_dropped++;
 		return 1;
 	}
@@ -983,13 +1160,11 @@ static int loongson_canfd_rx(struct net_device *ndev)
 	else
 		can_frame_set_cc_len(ccf, dlc, priv->can.ctrlmode);
 
-	for (i = 0; i < dbcnt; i += LOONGSON_CANFD_DW_BYTE) {
-		regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
-		put_unaligned_le32(data, cfd->data + i);
-	}
+	for (i = 0; i < dbcnt; i += LOONGSON_CANFD_DW_BYTE)
+		put_unaligned_le32(priv->get_rx_data(priv), cfd->data + i);
 
 	while (unlikely(i < dbcnt)) {
-		regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
+		priv->get_rx_data(priv);
 		i += LOONGSON_CANFD_DW_BYTE;
 	}
 
@@ -1204,7 +1379,7 @@ static int loongson_canfd_rx_napi(struct napi_struct *napi, int quota)
 	u32 sts;
 
 	while (work_done < quota) {
-		if (loongson_canfd_rxbuf_empty(priv)) {
+		if (priv->get_rxbuf_empty(priv)) {
 			no_more_data = true;
 			break;
 		}
@@ -1242,9 +1417,15 @@ static int loongson_canfd_rx_napi(struct napi_struct *napi, int quota)
 
 	if (no_more_data) {
 		if (napi_complete_done(napi, work_done)) {
-			regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, REG_INT_STAT_RBNEI);
-			regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK,
-				     REG_INT_STAT_RBNEI << 16);
+			int int_ena;
+
+			if (priv->rx_ch)
+				int_ena = REG_INT_STAT_DMADI;
+			else
+				int_ena = REG_INT_STAT_RBNEI;
+
+			regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, int_ena);
+			regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, (int_ena << 16));
 		}
 	}
 
@@ -1327,9 +1508,10 @@ static irqreturn_t loongson_canfd_interrupt(int irq, void *dev_id)
 				return IRQ_NONE;
 		}
 
-		if (isr & REG_INT_STAT_RBNEI) {
-			regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, REG_INT_STAT_RBNEI);
-			regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, REG_INT_STAT_RBNEI);
+		imask = priv->rx_ch ? REG_INT_STAT_DMADI : REG_INT_STAT_RBNEI;
+		if (isr & imask) {
+			regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, imask);
+			regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, imask);
 			napi_schedule(&priv->napi);
 		}
 
@@ -1572,18 +1754,40 @@ static int loongson_canfd_probe(struct platform_device *pdev)
 	ndev->ethtool_ops = &loongson_canfd_ethtool_ops;
 	SET_NETDEV_DEV(ndev, dev);
 
+	priv->get_rx_data = loongson_canfd_rxdata_poll;
+	priv->get_rxbuf_empty = loongson_canfd_rxbuf_empty_poll;
+
+	priv->rx_ch = dma_request_chan(dev, "rx");
+	if (PTR_ERR(priv->rx_ch) == -EPROBE_DEFER) {
+		ret = -EPROBE_DEFER;
+		goto err_candev_free;
+	}
+
+	if (IS_ERR(priv->rx_ch)) {
+		dev_warn(dev, "DMA not available, falling back to poll mode\n");
+		priv->rx_ch = NULL;
+	}
+
+	ret = loongson_canfd_rxdma_init(ndev);
+	if (!ret) {
+		priv->get_rx_data = loongson_canfd_rxdata_dma;
+		priv->get_rxbuf_empty = loongson_canfd_rxbuf_empty_dma;
+		priv->rx_rd_ptr_dist = LOONGSON_CANFD_RXDMA_NUM;
+	}
 	netif_napi_add(ndev, &priv->napi, loongson_canfd_rx_napi);
 
 	ret = register_candev(ndev);
 	if (ret) {
 		dev_err(dev, "register_candev failed with %d\n", ret);
-		goto err_candev_free;
+		goto err_napi_del;
 	}
 
 	return 0;
 
-err_candev_free:
+err_napi_del:
 	netif_napi_del(&priv->napi);
+	loongson_canfd_rxdma_remove(priv, &pdev->dev);
+err_candev_free:
 	free_candev(ndev);
 	return ret;
 }
@@ -1599,6 +1803,7 @@ static void loongson_canfd_remove(struct platform_device *pdev)
 
 	unregister_candev(ndev);
 	netif_napi_del(&priv->napi);
+	loongson_canfd_rxdma_remove(priv, &pdev->dev);
 	free_candev(ndev);
 }
 
@@ -1618,6 +1823,7 @@ static struct platform_driver loongson_canfd_driver = {
 };
 module_platform_driver(loongson_canfd_driver);
 
+MODULE_SOFTDEP("pre: loongson2-apb-cmc-dma");
 MODULE_AUTHOR("Loongson Technology Corporation Limited");
 MODULE_DESCRIPTION("Loongson CAN-FD Controller driver");
 MODULE_LICENSE("GPL");
Signature · 1 line
2.52.0
#4 lore raw

Re: [PATCH v8 2/2] can: loongson_canfd: Add RXDMA support

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [High] The cyclic DMA buffer wrap detection logic incorrectly interprets a normal buffer wrap as an overflow, silently discarding valid unread CAN frames.
- [High] The lockless NAPI DMA ring buffer logic reads variable-length CAN frames without verifying that the DMA engine has transferred the entire frame, leading to CPU over-reads of uninitialized or stale memory.
- [High] The cyclic DMA transfer is started during driver probe and runs continuously, leading to stream corruption across interface resets.

Pre-existing issues:
- [Critical] Missing payload length constraint for classic CAN frames leads to a heap buffer overflow in the SKB linear data area.
- [High] Malformed or invalid CAN frames cause permanent FIFO/DMA buffer desynchronization and an infinite NAPI polling loop by returning early without draining the invalid frame payload. --

commit 27376fa7494d4080cdce1ee73b4800fd7a987b82
Author: Binbin Zhou <[email protected]>

can: loongson_canfd: Add RXDMA support

Extend the driver with an optional DMA engine support for the receive path. When a DMA channel is available, the RX buffer is mapped to a cyclic DMA transfer that continuously fills a pre-allocated ring buffer. The driver then reads incoming CAN frames directly from the DMA buffer, improving performance under high traffic.

Quoted 5 lines
drivers/net/can/loongson_canfd.c +0 -0
index 6c983987a1208..7fb28bd00c14c 100644
@@ -523,12 +543,166 @@ static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev,

[ ... ]

Quoted 13 lines
drivers/net/can/loongson_canfd.c +13 -0
+static bool loongson_canfd_rxbuf_empty_dma(struct loongson_canfd_priv *priv)
+{
+	struct dma_tx_state state;
+	enum dma_status status;
+	u32 wptr_dist;
+
+	status = dmaengine_tx_status(priv->rx_ch, priv->rx_cookie, &state);
+	if (status != DMA_IN_PROGRESS)
+		return true;
+
+	/* Convert residue from bytes to number of 32-bit words */
+	wptr_dist = state.residue / DMA_SLAVE_BUSWIDTH_4_BYTES;
+

drivers/net/can/loongson_canfd.c static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, · line 596

Quoted 10 lines
drivers/net/can/loongson_canfd.c +10 -0
+	/*
+	 * wptr_dist is the write pointer's distance from the buffer end.
+	 * It starts at BUFFER_SIZE (when write pointer is at start) and
+	 * decreases to 0 (when write pointer reaches end), then wraps back.
+	 * priv->rx_rd_ptr_dist is the read pointer's distance from the end.
+	 * If write pointer wraps past read pointer, wptr_dist becomes larger,
+	 * indicating data loss, so we advance read pointer to write position.
+	 */
+	if (priv->rx_rd_ptr_dist < wptr_dist)
+		priv->rx_rd_ptr_dist = wptr_dist;

[Severity: High]
Does this logic mistakenly trigger on every normal buffer wrap-around?

Both wptr_dist and rx_rd_ptr_dist track the pointer's distance from the end of the buffer, decreasing towards 0. When the DMA write pointer completes a cycle and wraps back to the beginning, wptr_dist resets to its maximum value.

If the read pointer has not yet wrapped, the condition priv->rx_rd_ptr_dist < wptr_dist unexpectedly evaluates to true during a normal wrap-around.

The driver misinterprets this as the write pointer overtaking the read pointer and forcefully sets priv->rx_rd_ptr_dist = wptr_dist, which will erroneously skip all unread valid data remaining at the end of the buffer.

Conversely, if a true overflow occurs, wptr_dist is smaller, evaluating to false and failing to detect the overflow.

drivers/net/can/loongson_canfd.c

Quoted 2 lines
drivers/net/can/loongson_canfd.c +2 -0
+
+	return priv->rx_rd_ptr_dist == wptr_dist;

[Severity: High]
Could this cause the CPU to over-read uninitialized or stale memory?

This function returns false as soon as the DMA engine writes a single 32-bit word. Because the DMA engine transfers data asynchronously and word-by-word (or in small bursts), it may not have finished writing the full variable-length CAN frame when this function signals that data is ready.

The CPU will read ahead of the DMA write pointer in loongson_canfd_rx(), fetching stale memory, reading garbage payload or frame lengths, and permanently corrupting the stream pointer.

Quoted 1 line

+}

[ ... ]

Quoted 29 lines
drivers/net/can/loongson_canfd.c +29 -0
+static int loongson_canfd_rxdma_init(struct net_device *ndev)
+{
+	struct loongson_canfd_priv *priv = netdev_priv(ndev);
+	struct dma_async_tx_descriptor *desc = NULL;
+	struct device *dev = ndev->dev.parent;
+	struct dma_slave_config config;
+	int ret;
+
+	if (!priv->rx_ch)
+		return -EINVAL;
+
+	priv->rx_buf = dma_alloc_coherent(dev, LOONGSON_CANFD_RXBUF_SZ,
+					  &priv->rx_dma_buf, GFP_KERNEL);
+	if (!priv->rx_buf) {
+		dma_release_channel(priv->rx_ch);
+		priv->rx_ch = NULL;
+		return -ENOMEM;
+	}
+
+	memset(&config, 0, sizeof(config));
+	config.src_addr = priv->res->start + LOONGSON_CANFD_RX_DATA;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+
+	ret = dmaengine_slave_config(priv->rx_ch, &config);
+	if (ret < 0) {
+		netdev_err(ndev, "RX DMA channel config failed\n");
+		goto err_config;
+	}
+

drivers/net/can/loongson_canfd.c static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, · line 667

Quoted 10 lines
drivers/net/can/loongson_canfd.c +10 -0
+	desc = dmaengine_prep_dma_cyclic(priv->rx_ch, priv->rx_dma_buf,
+					 LOONGSON_CANFD_RXBUF_SZ, LOONGSON_CANFD_RXBUF_SZ,
+					 DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		netdev_err(ndev, "RX DMA cyclic transaction preparation failed\n");
+		ret = -EBUSY;
+		goto err_config;
+	}
+
+	priv->rx_cookie = dmaengine_submit(desc);

[Severity: High]
Could starting the DMA engine here during probe and running it continuously lead to stream corruption across interface resets?

When the interface is brought down via loongson_canfd_chip_stop(), the CAN controller stops and resets, but the DMA engine remains active with its current pointers.

When brought back up, the CAN controller is reset, but the DMA pointers and CPU read pointers are not reset. Because the hardware FIFO was flushed but the DMA buffer was not, the newly received CAN frames are appended to whatever partial frame was left in the DMA buffer before the reset.

Will this splice frames together and cause permanent stream desynchronization the first time the interface recovers from a bus-off condition or is manually reset?

Quoted 1 line
drivers/net/can/loongson_canfd.c +1 -0
+	ret = dma_submit_error(priv->rx_cookie);

[ ... ]

Quoted 18 lines
drivers/net/can/loongson_canfd.c +3 -3
@@ -929,14 +1106,14 @@ static int loongson_canfd_rx(struct net_device *ndev)
 {
 	struct loongson_canfd_priv *priv = netdev_priv(ndev);
 	struct net_device_stats *stats = &ndev->stats;
-	u32 frame_hdr0, frame_hdr1, dlc, rwcnt, dbcnt, i, data;
+	u32 frame_hdr0, frame_hdr1, dlc, rwcnt, dbcnt, i;
 	struct canfd_frame *cfd;
 	struct can_frame *ccf;
 	struct sk_buff *skb;
 	bool is_can_fd;
 
-	regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &frame_hdr0);
-	regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &frame_hdr1);
+	frame_hdr0 = priv->get_rx_data(priv);
+	frame_hdr1 = priv->get_rx_data(priv);
 
 	rwcnt = FIELD_GET(REG_FRAME_META1_RWCNT, frame_hdr1);
 	if (rwcnt < 2)
Quoted 1 line

return -EAGAIN;

[Severity: High]
This is a pre-existing issue, but does this early return cause permanent FIFO or DMA buffer desynchronization?

If the parsed header yields rwcnt < 2, the function returns -EAGAIN early. This aborts processing but leaves the read pointer (either the hardware FIFO pop count or the DMA read index) partially advanced by only 2 words.

The remaining payload words of the invalid frame are left in the buffer or FIFO. On the next NAPI poll, the driver will mistakenly read these leftover payload words as if they were a new header, resulting in permanent stream desynchronization.

Because the unread payload remains in the buffer, the empty check continuously returns true, triggering an interrupt storm and an infinite NAPI loop that hangs the CPU.

drivers/net/can/loongson_canfd.c

Quoted 4 lines
drivers/net/can/loongson_canfd.c +0 -0
 	dbcnt = (rwcnt - 2) * LOONGSON_CANFD_DW_BYTE;
 	if (dbcnt > CANFD_MAX_DLEN)
 		return -EAGAIN;

[Severity: High]
This is a pre-existing issue, but could this early return also cause permanent buffer desynchronization and NAPI loops, just like the rwcnt < 2 case above, because it fails to drain the remainder of the malformed frame?

Quoted 3 lines
drivers/net/can/loongson_canfd.c +0 -0
 	is_can_fd = !!(frame_hdr1 & REG_FRAME_META1_FDF);
 	if (is_can_fd) {

[ ... ]

drivers/net/can/loongson_canfd.c static int loongson_canfd_rx(struct net_device *ndev) · line 1164

Quoted 10 lines
drivers/net/can/loongson_canfd.c +2 -4
@@ -983,13 +1160,11 @@ static int loongson_canfd_rx(struct net_device *ndev)
 	else
 		can_frame_set_cc_len(ccf, dlc, priv->can.ctrlmode);
 
-	for (i = 0; i < dbcnt; i += LOONGSON_CANFD_DW_BYTE) {
-		regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
-		put_unaligned_le32(data, cfd->data + i);
-	}
+	for (i = 0; i < dbcnt; i += LOONGSON_CANFD_DW_BYTE)
+		put_unaligned_le32(priv->get_rx_data(priv), cfd->data + i);

[Severity: Critical]
This is a pre-existing issue, but does this loop cause a heap buffer overflow in cfd->data for classic CAN frames?

The code checks if dbcnt > CANFD_MAX_DLEN (64 bytes), but fails to ensure dbcnt <= CAN_MAX_DLEN (8 bytes) when processing a classic CAN frame.

For classic CAN frames, alloc_can_skb() allocates an SKB containing exactly a 16-byte struct can_frame.

If dbcnt exceeds 8 (which happens when the parser is desynchronized by the DMA race conditions or corrupted hardware state), this loop copies up to 64 bytes into cfd->data. This writes up to 56 bytes past the end of the allocated 16-byte buffer, triggering an overflow in the SKB's linear data area.

[Severity: High]
Could this unconditional read of the payload read ahead of the DMA write pointer?

Because loongson_canfd_rxbuf_empty_dma() signals data ready when the first word is written, the DMA engine may not have transferred these payload words yet. This will read garbage payload data and corrupt the stream state.

Quoted 6 lines
drivers/net/can/loongson_canfd.c +1 -1
 	while (unlikely(i < dbcnt)) {
-		regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
+		priv->get_rx_data(priv);
 		i += LOONGSON_CANFD_DW_BYTE;
 	}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2