Add Loongson CAN-FD controller driver
v7 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
=========
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 | 1913 ++++++++++++++++++++++++++++++ 4 files changed, 1932 insertions(+) create mode 100644 drivers/net/can/loongson_canfd.c
base-commit: 3bd438a58e910db5dc369aa25dfed1fc95f1b596
Signature · 1 line
2.52.0
PATCH v7 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 | 1722 ++++++++++++++++++++++++++++++ 4 files changed, 1741 insertions(+) create mode 100644 drivers/net/can/loongson_canfd.c
MAINTAINERS +7 -0
@@ -15195,6 +15195,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
@@ -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
@@ -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 +1722 -0
1,722 lines not on this page · raw message
Signature · 1 line
2.52.0
[email protected]
AI reviewRe: [PATCH v7 1/2] can: Add Loongson CAN-FD controller driver
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider: - [Critical] Out-of-bounds write in RX path when handling classical CAN 2.0 frames with unusually large…
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] Out-of-bounds write in RX path when handling classical CAN 2.0 frames with unusually large length values.
- [High] Returning -EAGAIN on invalid RX frames leaves the hardware FIFO blocked, causing permanent desynchronization and an interrupt storm.
- [High] The driver uses undefined lowercase field_get() and field_prep() macros, and dynamic masks would break the build even with standard bitfield macros.
- [High] TX interrupt handler clears the interrupt status conditionally and at the wrong time, causing lost interrupts and infinite loops.
- [High] Explicitly clearing the error interrupt status at the end of loongson_canfd_err_interrupt() creates a race condition.
- [High] Incorrect logic in loongson_canfd_txbuf_freed() artificially limits the 8-buffer hardware TX ring to a queue depth of 1.
- [Medium] The driver mutates the netlink-provided can_bittiming structure during hardware calculation.
- [Medium] Directly casting the byte array payload to a 32-bit pointer for regmap access reverses the byte order on Big Endian systems. --
--- Patch [1]: [PATCH v7 1/2] can: Add Loongson CAN-FD controller driver --- commit 8ea91a6644d61e09bfa3d704e0a569716fdc82b2
Author: Binbin Zhou <[email protected]>
Subject: can: Add Loongson CAN-FD controller driver
This commit adds a new network device driver for the Loongson CAN-FD controller found in Loongson-2K SoCs. The driver supports CAN 2.0, CAN-FD, loopback, and various other controller modes, using NAPI for receive and offering TX echo support.
drivers/net/can/loongson_canfd.c line 465
Quoted 9 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -462,11 +462,4 @@static bool loongson_canfd_txbuf_freed (struct loongson_canfd_priv *priv) {return !regmap_test_bits (priv->regmap,LOONGSON_CANFD_TX_STAT ,REG_TX_STAT_BRP ); }
[Severity: High]
Does this logic artificially limit the 8-buffer hardware TX ring to a queue depth of 1?
Since regmap_test_bits() returns true if any bits in REG_TX_STAT_BRP are set, the negation here returns true only when all 8 buffers are perfectly idle.
This would cause loongson_canfd_start_xmit() to prematurely stop the queue and falsely drop frames when even a single buffer is transmitting:
loongson_canfd_start_xmit() {
...
if (unlikely(!loongson_canfd_txbuf_freed(priv))) {
netif_stop_queue(ndev);
netdev_err(ndev, "BUG!, no TXB free when queue awake!\n");
return NETDEV_TX_BUSY;
}
...
}[ ... ]
Quoted 11 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -484,10 +484,7 @@static enum
loongson_canfd_txbuf_stsloongson_canfd_get_txbuf_sts(struct loongson_canfd_priv *priv, u8 buf_id){ u32 sts, mask; mask = REG_TX_STAT_BS_TX_MASK_BASE << 2 * buf_id; regmap_read(priv->regmap, LOONGSON_CANFD_TX_STAT, &sts); return field_get(mask, sts); }
[Severity: High]
Can this compile successfully?
The driver uses an undefined lowercase field_get() macro here, and a lowercase field_prep() in loongson_canfd_set_txbuf_cmd().
Even if corrected to the standard FIELD_GET() and FIELD_PREP() macros, the build will still fail. The bitfield macros enforce a build bug check requiring the mask to be a compile-time constant, but here the mask is dynamically derived from the runtime variable buf_id.
[ ... ]
Quoted 11 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -711,27 +711,10 @@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 btr =0 ;/* * The timing calculation functions have only constraints on tseg1, * which is prop_seg + phase1_seg combined. * tseg1 is then split in half and stored into prog_seg and phase_seg1.
Quoted 7 lines
* In Loongson CAN-FD, PROP is 7 bits wide but PH1 only 6, so we must
* re-distribute the values here.
*/
if (bt->phase_seg1 > ph1_max) {
bt->prop_seg += bt->phase_seg1 - ph1_max;
bt->phase_seg1 = ph1_max;
}[Severity: Medium]
Does this mutate the netlink-provided can_bittiming structure?
By directly modifying bt->prop_seg and bt->phase_seg1 via the bt pointer, the driver modifies the shared persistent structure (priv->can.bittiming or priv->can.fd.data_bittiming) instead of a local copy.
This can corrupt the core CAN state visible to userspace, leading to incorrect calculations on subsequent interface restarts.
[ ... ]
Quoted 4 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -985,41 +985,15 @@static bool loongson_canfd_insert_frame (struct loongson_canfd_priv *priv,const struct canfd_frame *cf,u8 buf,bool isfdf) {
[ ... ]
Quoted 9 lines
/* Write Data payload */
for (unsigned int i = 0; i < cf->len; i += LOONGSON_CANFD_DW_BYTE) {
regmap_write(priv->regmap,
LOONGSON_CANFD_TX_DATA_1 + LOONGSON_CANFD_FRAME_DB_1 + i,
*(u32 *)(cf->data + i));
}
return true;
}[Severity: Medium]
Will this silently corrupt the CAN frame payload on big endian architectures?
Directly casting the byte array payload to a 32-bit pointer reverses the byte order when the host-endian u32 is written out on big endian systems.
This also violates strict aliasing rules. The same issue appears in the receive path in loongson_canfd_rx().
[ ... ]
Quoted 3 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -1114,64 +1114,14 @@static int loongson_canfd_rx (struct net_device *ndev) {
[ ... ]
Quoted 8 lines
/* Number of characters received */ rwcnt = FIELD_GET(REG_FRAME_META1_RWCNT, meta1); if (rwcnt < 2) return -EAGAIN; dbcnt = (rwcnt - 2) * LOONGSON_CANFD_DW_BYTE; if (dbcnt > CANFD_MAX_DLEN) return -EAGAIN;
[Severity: High]
Does returning -EAGAIN here leave the hardware FIFO permanently blocked?
Since the first two meta words were already popped from the FIFO via regmap_read(), returning without draining the remaining payload words leaves the hardware FIFO completely out of sync.
This can lead to a permanent device lockup and an endless interrupt NAPI storm.
[ ... ]
Quoted 8 lines
/* Flexible Data-rate Format */
is_can_fd = !!(meta1 & 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;
}[ ... ]
Quoted 3 lines
/* Copy payload */ for (i = 0; i < dbcnt; i += LOONGSON_CANFD_DW_BYTE) regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, (u32 *)(cfd->data + i));
[Severity: Critical]
Can this loop cause an out-of-bounds write if a classical CAN 2.0 frame is received with an unusually large length?
If a corrupted bus frame or hardware glitch causes a classical CAN 2.0 frame to have an rwcnt indicating more than 2 payload words, alloc_can_skb() will only allocate an 8-byte data buffer.
However, the loop will blindly copy up to 64 bytes into cfd->data, corrupting skb_shared_info and adjacent memory.
[ ... ]
Quoted 3 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -1275,103 +1275,14 @@static void loongson_canfd_err_interrupt (struct net_device *ndev,u32 isr) {
[ ... ]
Quoted 6 lines
if (skb) netif_rx(skb); regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, mask); regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, mask << 16); }
[Severity: High]
Does explicitly clearing the error interrupt status here create a race condition?
The main handler loongson_canfd_interrupt() already correctly clears error interrupts before calling this function. Redundantly writing the mask back to LOONGSON_CANFD_INT_STAT at the very end of this function creates a window for lost interrupts.
Any new error asserted by the hardware during the execution of this handler will be instantly cleared and ignored, dropping crucial events like bus-off or arbitration lost.
[ ... ]
Quoted 3 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -1480,59 +1480,17 @@static void loongson_canfd_tx_interrupt (struct net_device *ndev) {
[ ... ]
Quoted 12 lines
loongson_canfd_set_txbuf_cmd(ndev, TXT_CMD_SR_CLEAN, buf_id);
}
/*
* If no buffers were processed this time, we cannot clear - that would introduce
* a race condition.
*/
if (processed) {
/*
* Clear the interrupt again. We do not want to receive again interrupt
* for the buffer already handled. If it is the last finished one then
* it would cause log of spurious interrupt.Quoted 8 lines
*/ regmap_write(priv->regmap, LOONGSON_CANFD_INT_STAT, REG_INT_STAT_TXBHCI); } /* Check if at least one TX buffer is free */ if (loongson_canfd_txbuf_freed(priv)) netif_wake_queue(ndev); }
[Severity: High]
Does conditionally clearing the interrupt status at the end lead to lost interrupts or infinite loops?
If a new TX buffer completes after its buf_id is checked in the loop, but before the end of the function, the regmap_write unconditionally clears REG_INT_STAT_TXBHCI, erasing the new completion notification.
Additionally, if the handler runs but no buffers are ready (processed is false), the interrupt status is never cleared. This will cause loongson_canfd_interrupt() to loop repeatedly and potentially shut down the device.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH v7 2/2 can: loongson_canfd: Add RXDMA support
In reply to Binbin ZhouExtend 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.
The DMA mode is completely transparent to the under networking stack; the driver automatically selects the appropriate RX data reading method based on the availability of the DMA channel.
--- drivers/net/can/Kconfig | 2 +- drivers/net/can/loongson_canfd.c | 231 ++++++++++++++++++++++++++++--- 2 files changed, 212 insertions(+), 21 deletions(-)
drivers/net/can/Kconfig +1 -1
@@ -188,7 +188,7 @@ config CAN_KVASER_PCIEFD config CAN_LOONGSON_CANFD tristate "Loongson CAN-FD controller" - depends on HAS_IOMEM + depends on HAS_IOMEM && (LOONGSON2_APB_CMC_DMA || COMPILE_TEST) select REGMAP_MMIO help This is a canfd driver switch for the Loongson platform,
drivers/net/can/loongson_canfd.c +211 -20
358 lines not on this page · raw message
Signature · 1 line
2.52.0
[email protected]
AI reviewRe: [PATCH v7 2/2] can: loongson_canfd: Add RXDMA support
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] NAPI RX polling races with DMA engine word-by-word transfers, causing frame corruption and permanent stream desynchronization.
- [High] Spurious reads and desynchronization due to logic flaw when DMA residue reaches zero.
- [Medium] Silent data loss and misalignment due to lack of RX DMA cyclic buffer overflow detection.
- [Medium] Missing dma_rmb() causes reads of stale coherent DMA memory.
Pre-existing issues:
- [Critical] Heap buffer overflow when receiving Classical CAN frames due to missing length validation.
- [High] Incomplete consumption of invalid frames leads to permanent RX stream desynchronization. --
--- Patch [2]: [PATCH v7 2/2] can: loongson_canfd: Add RXDMA support --- commit c004fc59a1568e0e67b052c8c64387941f3c4086
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 named rx is available, the RX buffer is mapped to a cyclic DMA transfer that continuously fills a ring buffer.
Quoted 4 lines
drivers/net/can/loongson_canfd.c +0
-0
[ ... ]
drivers/net/can/loongson_canfd.c static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, · line 548
Quoted 6 lines
drivers/net/can/loongson_canfd.c +5
-0@@ -524,13 +544,150 @@ static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, +static u32 loongson_canfd_get_rxdata_in_dma (struct loongson_canfd_priv *priv) +{ +u32 data =0 ; + + data = priv->rx_buf[LOONGSON_CANFD_RXDMA_NUM - priv->last_res--];
[Severity: Medium]
Does reading from the DMA buffer in loongson_canfd_get_rxdata_in_dma() require a read memory barrier? On weakly ordered architectures like LoongArch, the CPU might speculatively prefetch the coherent DMA buffer before the MMIO status check in loongson_canfd_get_rxbuf_empty_in_dma() confirms the data is ready. Could a dma_rmb() be needed to order this memory read after the MMIO status check?
Quoted 6 lines
drivers/net/can/loongson_canfd.c +6
-0+if (!priv->last_res) + priv->last_res =LOONGSON_CANFD_RXDMA_NUM ; + +return data; +} +
drivers/net/can/loongson_canfd.c static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, · line 579
Quoted 10 lines
drivers/net/can/loongson_canfd.c +10
-0+static bool loongson_canfd_get_rxbuf_empty_in_dma (struct loongson_canfd_priv *priv) +{ +struct dma_tx_state state; +enum dma_status status; + + status =dmaengine_tx_status (priv->rx_ch, priv->rx_cookie, &state); +if (status !=DMA_IN_PROGRESS ) +return true; + +return priv->last_res == (state.residue /DMA_SLAVE_BUSWIDTH_4_BYTES );
[Severity: High]
Since priv->last_res wraps directly from 1 to LOONGSON_CANFD_RXDMA_NUM
(256) and never becomes 0, what happens when the cyclic DMA transfer reaches the end of its period and state.residue is 0? Will this cause the empty check to evaluate to false (since 256 != 0), making the driver incorrectly believe data is available when the buffer is actually empty?
[Severity: High]
Does this logic assume that a full frame is available as soon as the DMA residue differs from last_res? Because the DMA engine writes word-by-word, the empty check will return false as soon as a single 32-bit word is written. Will the subsequent unconditional full frame read in loongson_canfd_rx() read ahead of the DMA controller into stale memory, parsing incorrect lengths and permanently losing synchronization with the cyclic buffer?
[Severity: Medium]
Is there a mechanism to detect if the DMA write pointer has lapped the read pointer under high traffic? Since the driver only checks for exact equality of the read and write pointers (last_res == residue / 4), it seems it cannot distinguish between an empty buffer and a buffer that has been lapped by exactly 256 words. Will this lead to the CPU reading from the middle of an overwritten frame without noticing?
[ ... ]
Quoted 18 lines
drivers/net/can/loongson_canfd.c +3
-3@@ -957,14 +1117,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 meta0, meta1, dlc, rwcnt, dbcnt, i, data; +u32 meta0, meta1, 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 , &meta0); -regmap_read (priv->regmap,LOONGSON_CANFD_RX_DATA , &meta1); + meta0 = priv->get_rx_data (priv); + meta1 = priv->get_rx_data (priv);/* Number of characters received */ rwcnt =FIELD_GET (REG_FRAME_META1_RWCNT , meta1);
drivers/net/can/loongson_canfd.c
Quoted 6 lines
drivers/net/can/loongson_canfd.c +0
-0if (rwcnt <2 )return -EAGAIN ; dbcnt = (rwcnt -2 ) *LOONGSON_CANFD_DW_BYTE ;if (dbcnt >CANFD_MAX_DLEN )return -EAGAIN ;
[Severity: High]
This is a pre-existing issue, but by returning -EAGAIN early in loongson_canfd_rx() when rwcnt < 2 or dbcnt > CANFD_MAX_DLEN, does the driver fail to drain the remaining invalid payload words from the RX source? If the payload is not consumed, will the next read attempt to parse the leftover payload as the metadata headers for the next frame, permanently desynchronizing the RX stream? This seems easier to trigger now with the new DMA race conditions.
[ ... ]
drivers/net/can/loongson_canfd.c static int loongson_canfd_rx(struct net_device *ndev) · line 1182
Quoted 4 lines
drivers/net/can/loongson_canfd.c +1
-1/* Copy payload */ for (i =0 ; i < dbcnt; i +=LOONGSON_CANFD_DW_BYTE ) -regmap_read (priv->regmap,LOONGSON_CANFD_RX_DATA , (u32 *)(cfd->data + i)); + *(u32 *)(cfd->data + i) = priv->get_rx_data (priv);
[Severity: Critical]
This is a pre-existing issue, but does this loop cause a heap buffer overflow when receiving Classical CAN frames? In loongson_canfd_rx(), for Classical CAN frames (is_can_fd == false), the allocated skb only has an 8-byte data array. However, the earlier bounds check only validates that dbcnt <= CANFD_MAX_DLEN (64). Could this write up to 56 bytes past the end of the can_frame into skb_shared_info, potentially leading to memory corruption?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2