Re: [PATCH 2/3] [v4, net-next] net: ethernet: ti-cpsw: fix linking built-in code to modules

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [Critical] Shared global function pointer `cpsw_slave_index` causes use-after-free and out-of-bounds access across modules.
- [High] Shared global mutable array `ale_controls` causes hardware state corruption across different devices.
- [High] Userspace ABI breakage due to silent module rename of `ti_davinci_emac.ko` to `davinci_emac.ko`.
- [Low] Swapped EXPORT_SYMBOL_GPL macros for `cpsw_hwtstamp_set` and `cpsw_hwtstamp_get` under `!CONFIG_TI_CPTS`.
--

Note: The format of this review is altered to provide a free-form summary
of findings due to quoting/recitation restrictions.

commit bddbb0ecb5a5bfa6e8eb526f950b6ccd9620d793
Author: Arnd Bergmann <arnd@arndb.de>
net: ethernet: ti-cpsw: fix linking built-in code to modules

This commit changes the build system to create separate standalone modules
for shared CPSW driver components. It aims to resolve Kbuild warnings about
objects being linked into multiple modules by exporting symbols and creating
shared modules like ti-cpsw-common.ko, ti-cpsw-priv.ko, and ti-cpsw-ale.ko.
Link: https://lore.kernel.org/lkml/20240417084400.3034104-1-arnd@kernel.org/

Looking at the extraction of cpsw_priv.c into the shared ti-cpsw-priv.ko
module, does this introduce a use-after-free regression on the global
cpsw_slave_index function pointer?

This pointer is now shared system-wide across multiple drivers. If both
the older and newer cpsw drivers initialize and overwrite this global
pointer with their own module-specific functions, will unloading the
last loaded module leave a dangling pointer?

If the remaining driver attempts to access it, could it lead to a panic
or an out-of-bounds array access on cpsw->slaves?

Similarly, in the new ti-cpsw-ale.ko module, can the shared ale_controls
global array cause state corruption across different devices?

The ale_controls array is mutated at runtime during cpsw_ale_create()
based on device-specific parameters. Since it is no longer isolated per
driver, if a device like the am65-cpsw-nuss probes and mutates the global
array offsets, do subsequent standard CPSW devices end up using these
incorrect offsets?

In the drivers/net/ethernet/ti/Makefile, does changing the module target
from ti_davinci_emac.o to davinci_emac.o cause a userspace ABI regression?

Because the object name changed, Kbuild will now produce davinci_emac.ko
instead of the historic ti_davinci_emac.ko. Could this break modprobe
configurations or initramfs scripts that expect the old module name?

Finally, in cpsw_priv.c under the !CONFIG_TI_CPTS block, this isn't a bug,
but are the export macros swapped?

The EXPORT_SYMBOL_GPL(cpsw_hwtstamp_set) macro is placed directly after
the cpsw_hwtstamp_get() stub, and EXPORT_SYMBOL_GPL(cpsw_hwtstamp_get)
is placed after the cpsw_hwtstamp_set() stub.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260402184726.3746487-1-arnd@kernel.org?part=2