Re: [PATCH 01/15] yunsilicon: add PCI core driver skeleton

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] mm: invalid kfree() on memory allocated with kvzalloc_obj()
- [High] hw: endian-dependent C bitfields in hardware descriptors corrupt data
- [High] hw: native host-endian types used in hardware DMA structures
- [High] hw: implicit compiler padding corrupts hardware command layouts
- [Medium] pci: missing dma_set_mask() forces 32-bit streaming DMA fallback
--

commit 2f6b2de0263e58c06ddcb030ee0e72ee0f685f39
Author: Xin Tian <tianx@yunsilicon.com>
Subject: yunsilicon: add PCI core driver skeleton

This commit adds the basic PCI core driver for the Yunsilicon XSC network
device. It includes Kconfig, Makefiles, register definitions, and basic PCI
probe/remove routines that enable the device and allocate the per-function
resource structure.

[Severity: High]
In drivers/net/ethernet/yunsilicon/xsc/pci/main.c, inside xsc_dev_res_init()
around line 105, xdev->dev_res is allocated using kvzalloc_obj(). However,
in xsc_dev_res_cleanup() at line 118, the memory is freed using kfree()
instead of kvfree(). Does this code potentially cause a kernel panic or SLUB
corruption during driver unbind or probe failure? If kvmalloc() falls back to
a vmalloc address because physically contiguous memory is unavailable, calling
kfree() on it violates the allocator API.

[Severity: Medium]
In drivers/net/ethernet/yunsilicon/xsc/pci/main.c within xsc_set_dma_caps()
at line 22, the code configures a 64-bit mask for coherent allocations via
dma_set_coherent_mask(), but omits dma_set_mask() for the streaming DMA mask.
Is the streaming DMA mask intentionally left at the 32-bit default? By not
setting it, all network packet buffers mapped via dma_map_single() or
dma_map_page() will be restricted to the lower 4GB of physical RAM. On 64-bit
systems with more than 4GB of RAM, this forces the networking stack to bounce
all network traffic through the SWIOTLB, which can cause massive performance
degradation and potential silent allocation failures if the SWIOTLB is
exhausted.

[Severity: High]
In drivers/net/ethernet/yunsilicon/xsc/common/xsc_core.h (for xsc_cqe at line
242, and xsc_send_wqe_ctrl_seg at line 99) and xsc_cmdq.h (for
xsc_cmd_layout at line 113), hardware descriptors mapped to device DMA memory
define C bitfields using types like __le32 and u8. Will these hardware
descriptor layouts break on big-endian architectures? The C standard dictates
that bitfield packing order is implementation-defined. On little-endian
systems, GCC packs bits from LSB to MSB, but on big-endian systems, from MSB
to LSB. Furthermore, mixing types like u8 and __le32 in bitfields causes
compiler-dependent alignment padding that bloats structural sizes. Compiling
this driver on a big-endian system might result in malformed commands and
silent data corruption. Fixed-size types with bitwise masks should likely be
used instead.

[Severity: High]
In drivers/net/ethernet/yunsilicon/xsc/common/xsc_cmd.h, several structures
passed to or read from the hardware via DMA or mailbox commands define fields
using native host types. For example, xsc_cq_context_ex uses u32 pa_num_ex
at line 196, xsc_event_linkinfo uses u32 and u64 for linkspeed and supported
at line 863, and xsc_hw_stats_rdma_pf uses u64 for statistics at line 927.
Can this cause data corruption on big-endian hosts? Hardware devices operate
with a fixed endianness, and using native host types will cause the CPU on
big-endian hosts to read or write the bytes without swapping them, producing
incorrect values. All fields in DMA-shared structures typically need explicit
__le32/__le64 or __be32/__be64 types.

[Severity: High]
Also in drivers/net/ethernet/yunsilicon/xsc/common/xsc_cmd.h, missing packed
attributes or explicit manual padding creates implicit holes in hardware
command and configuration structures. For instance, in struct xsc_hca_cap,
__be64 flags requires 8-byte alignment but follows u8 rsvd16[2], resulting in
6 bytes of implicit padding. In struct xsc_create_qp_request at line 313,
__be64 pas[] forces 4 bytes of padding after u8 rsvd. In struct xsc_rss_attr
at line 782, __be32 hash_tmpl forces 2 bytes of padding after __be16 rqn_num.
Does this implicit compiler padding misalign the hardware command layouts?
This padding shifts all subsequent fields transmitted to and from the
hardware, potentially corrupting the mailbox commands and breaking the ABI
contract with the firmware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012114352.2471317-1-tianx@yunsilicon.com?part=1