can: add arasan canfd controller support

Documentation/devicetree/bindings/net/can/arasan,canfd.yaml line 47

[Severity: Medium]
Should clock-names and reset-names be included in this required list? A device tree omitting these names might pass validation, but could cause the driver to fail during probe when requesting clocks or resets by name.

Also, should clocks and resets specify minItems: 2, since the definition expects two distinct items for each?

Documentation/devicetree/bindings/net/can/arasan,canfd.yaml

[Severity: Medium]
Should this use unevaluatedProperties: false instead? Since the schema includes can-controller.yaml via $ref earlier in the file, using additionalProperties:
false will prevent the inheritance of valid standard properties from the included schema.

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

drivers/net/can/arasan-canfd.c line 292

[Severity: Critical]
Can this cause a divide-by-zero crash when configuring the device in standard CAN 2.0 mode?

When the interface is brought up without CAN FD mode enabled, the CAN framework does not populate the fd.data_bittiming structure, leaving dbt->bitrate as 0.

drivers/net/can/arasan-canfd.c line 297

[Severity: High]
Does this driver completely bypass the CAN framework's calculated timing parameters?

The code derives cycle lengths directly from the clock frequency and assigns fixed proportions for timing segments instead of using the calculated values in bt like bt->prop_seg, bt->phase_seg1, and bt->sjw. This prevents userspace from configuring specific sample points via netlink.

[ ... ]

drivers/net/can/arasan-canfd.c line 405

[Severity: Medium]
Could an independent evaluation of these states cause a lower-priority state to overwrite a terminal state?

If the hardware latches both conditions simultaneously, the driver processes Bus Off, but then immediately processes Error Passive and overwrites the internal state to CAN_STATE_ERROR_PASSIVE, breaking the CAN state machine. Should this use an else if structure?

[ ... ]

drivers/net/can/arasan-canfd.c line 430

[Severity: High]
Does allocating tx_frame on the stack without initializing it leak kernel memory?

The code copies only cf->len bytes into the data array, leaving the remainder of the 64-byte array and the resv_0 field uninitialized. The entire 76-byte structure is then copied into the DMA-mapped buffer, potentially leaking uninitialized kernel stack data to the device.

drivers/net/can/arasan-canfd.c line 435

[Severity: Medium]
Are these CPU-native bitwise operations and assignments safe on big-endian architectures?

The DMA descriptors are populated using CPU-native types and directly copied to DMA memory. On big-endian systems, this data will be stored in big-endian format. If the hardware expects little-endian byte ordering, it will interpret swapped bits.

drivers/net/can/arasan-canfd.c line 442

[Severity: Medium]
Does this direct assignment write Linux internal CAN flags into the hardware descriptor?

cf->can_id contains internal flags such as CAN_EFF_FLAG and CAN_RTR_FLAG. Writing these directly into the hardware's TX descriptor field might corrupt hardware configuration bits. Should this be masked using CAN_EFF_MASK?

[ ... ]

drivers/net/can/arasan-canfd.c line 474

[Severity: Medium]
Is a dma_wmb() barrier needed here before writing to the HITXFIFO register?

Because writel_relaxed lacks memory ordering guarantees, the CPU can reorder the register write before the DMA memory writes complete, potentially causing the hardware to transmit stale data.

[ ... ]

drivers/net/can/arasan-canfd.c line 511

[Severity: Medium]
Can this lockless call to netif_stop_queue race with netif_wake_queue?

If a concurrent TX completion occurs right after arasan_canfd_start_xmit_queue releases the lock, the completion handler will free a slot and call netif_wake_queue. This lockless netif_stop_queue could then execute, overwriting the wake and deadlocking transmission.

drivers/net/can/arasan-canfd.c line 529

[Severity: High]
Does this bounds check fail to protect against negative index values?

The driver calculates the signed integer idx by subtracting priv->buf_dma from a hardware-provided address. If the hardware address is unexpectedly small, it can cause unsigned wrap-around, resulting in a negative value when cast to int. A negative value passes the idx > CAN_MAX_TX_QUEUE check. Additionally, the check uses > instead of >=, so idx == CAN_MAX_TX_QUEUE passes, causing an off-by-one out-of-bounds access.

[ ... ]

drivers/net/can/arasan-canfd.c line 553

[Severity: High]
Does this bounds check have the same off-by-one and negative index bypass issue as the TX path?

The idx calculation can produce a negative integer or exactly equal CAN_MAX_RX_QUEUE, both of which bypass this check and lead to out-of-bounds memory access.

[ ... ]

drivers/net/can/arasan-canfd.c line 569

[Severity: High]
Could returning 0 on allocation failure lead to an infinite softirq loop?

In the NAPI poll loop, work_done is incremented by the return value of this function. If this returns 0 under memory pressure, work_done is not advanced. Under continuous CAN traffic, work_done will never reach quota, and the poll loop will spin the CPU indefinitely in softirq context.

[ ... ]

drivers/net/can/arasan-canfd.c line 587

[Severity: High]
Does faulty operator precedence corrupt the CAN ID reconstruction here?

Because the bitwise OR operator has lower precedence than the left shift operator, the compiler evaluates this as:
A | ((B << 7) << 18)
This means the lower 7 bits of the standard ID are never shifted up by 18, corrupting bits 0-6 instead.

[ ... ]

drivers/net/can/arasan-canfd.c line 619

[Severity: Medium]
Is a dma_rmb() barrier needed here after reading USED_BUFS_PTR?

Without a read memory barrier, the CPU might reorder memory accesses and read stale RX descriptor data in arasan_canfd_rx before the hardware has fully written it.

drivers/net/can/arasan-canfd.c line 631

[Severity: High]
Does this TX abort case permanently leak TX queue slots?

This case does not extract the transmission index, free the socket buffer, or reclaim the queue slot. The SKB and TX queue slot are leaked, which will permanently stall transmission once the queue fills up. It also fails to increment work_done, risking a softirq stall similar to the RX allocation failure path.

[ ... ]

drivers/net/can/arasan-canfd.c line 716

[Severity: High]
Does enabling NAPI after hardware interrupts are unmasked create a deadlock window?

arasan_canfd_chip_start enables hardware interrupts. If an interrupt fires before napi_enable is called, the ISR will mask the interrupt and schedule a NAPI poll. Since NAPI isn't enabled yet, the poll is discarded and the interrupt remains masked permanently.

[ ... ]

drivers/net/can/arasan-canfd.c line 793

[Severity: Medium]
Can this truncate a 64-bit DMA address?

priv->buf_dma can be a 64-bit address on platforms without a restricted DMA mask, but it is written directly into a 32-bit register. If the address is above 4GB, the hardware will be directed to the wrong memory location.

[ ... ]

drivers/net/can/arasan-canfd.c line 833

[Severity: High]
Can a pending NAPI poll run concurrently with system suspend here?

This function stops the chip but fails to call napi_disable() and disable_irq() to wait for and terminate concurrent executions. pm_runtime_force_suspend will shut off the device clocks, meaning a concurrent NAPI poll could access powered down hardware and cause a kernel panic.

[ ... ]

drivers/net/can/arasan-canfd.c line 1014

[Severity: Medium]
Does pm_runtime_disable cancel the asynchronous suspend request?

Calling pm_runtime_disable immediately after an asynchronous pm_runtime_put cancels the pending suspend work. This means the suspend callback is never executed, leaving the hardware incorrectly powered on and leaking clocks. Should this use pm_runtime_put_sync instead?

[ ... ]

drivers/net/can/arasan-canfd.c line 1032

[Severity: Medium]
Does this also suffer from the cancelled asynchronous suspend issue?

unregister_candev invokes arasan_canfd_close, which performs an asynchronous pm_runtime_put. The immediate pm_runtime_disable cancels it, again leaking clocks and leaving the device powered on.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2