Re: [PATCH 08/11] ARM: mach-lpc: Remove NOMMU platform support

[sashiko-bot@kernel.org]

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

New issues:
- [High] Incorrect removal of an active MMU platform (LPC32xx) under the false premise of removing NOMMU platforms.
- [High] Incomplete Kconfig cleanup leaves dangling dependencies across the kernel, violating Kconfig subsystem rules.

Pre-existing issues:
- [High] Forcing the fallback stub for `lpc32xx_return_iram()` exposes a NULL pointer dereference and a zero-sized DMA allocation bug in `lpc_eth.c`.
--

commit 19639a4680fe193e3e1ece890c77d5f80c446705
Author: Frank Li <Frank.Li@nxp.com>

ARM: mach-lpc: Remove NOMMU platform support

Commercial users and hardware vendors migrated to Zephyr or other RTOS
solutions years ago, leaving the NOMMU platform support effectively
unused and unmaintained.

Remove the obsolete support to reduce maintenance burden and simplify the
NXP/Freescale platform code.

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 9187240a02db5..fe67d41f4a107 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig

[ ... ]

> @@ -401,8 +401,6 @@ source "arch/arm/mach-ixp4xx/Kconfig"
>  
>  source "arch/arm/mach-keystone/Kconfig"
>  
> -source "arch/arm/mach-lpc32xx/Kconfig"
> -

[Severity: High]
Does this removal leave dangling dependencies in other subsystems?

For example, drivers/net/ethernet/nxp/Kconfig has:
    depends on ARCH_LPC32XX || COMPILE_TEST

And arch/arm/Kconfig.debug has:
    depends on ARCH_LPC32XX

Since this patch deletes ARCH_LPC18XX and ARCH_LPC32XX but doesn't clean
up the references to them, will these become dead code that violate Kconfig
subsystem rules?

>  source "arch/arm/mach-mediatek/Kconfig"
>  
>  source "arch/arm/mach-meson/Kconfig"

[ ... ]

> diff --git a/arch/arm/mach-lpc32xx/Kconfig b/arch/arm/mach-lpc32xx/Kconfig
> deleted file mode 100644
> index 138599545c24c..0000000000000
> --- a/arch/arm/mach-lpc32xx/Kconfig
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -# SPDX-License-Identifier: GPL-2.0-only
> -
> -config ARCH_LPC32XX
> -	bool "NXP LPC32XX"
> -	depends on ARCH_MULTI_V5
> -	depends on CPU_LITTLE_ENDIAN
> -	select ARM_AMBA
> -	select CLKSRC_LPC32XX
> -	select CPU_ARM926T

[Severity: High]
Is LPC32XX actually a NOMMU platform?

Looking at the definition being removed here, it selects CPU_ARM926T and
depends on ARCH_MULTI_V5.

Since ARM926EJ-S features an MMU and runs standard Linux, does removing this
under the premise of removing NOMMU platforms accidentally delete an active,
MMU-equipped SoC?

[ ... ]

> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> deleted file mode 100644
> index 304ea61a07160..0000000000000
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ /dev/null

[ ... ]

> -static u32 iram_size;
> -u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> -{

[Severity: High]
This is a pre-existing issue, but does removing ARCH_LPC32XX permanently
expose a bug in the lpc32xx_return_iram() fallback stub?

When the platform is removed, include/linux/soc/nxp/lpc32xx-misc.h
forces the use of the dummy implementation:

    static inline u32 lpc32xx_return_iram(void __iomem **mapbase,
                                          dma_addr_t *dmaaddr)
    {
        *mapbase = NULL;
        *dmaaddr = 0;
        return 0;
    }

This unconditionally dereferences the pointer arguments.

In drivers/net/ethernet/nxp/lpc_eth.c:lpc_eth_drv_remove(), the driver
passes NULL for these arguments:

    if (!use_iram_for_net(&pldat->pdev->dev) ||
        pldat->dma_buff_size > lpc32xx_return_iram(NULL, NULL))

Will forcing the fallback stub cause a NULL pointer dereference here?

Additionally, since the stub returns 0, lpc_eth_drv_probe() sets the
buffer size to 0:

    if (pldat->dma_buff_size >
        lpc32xx_return_iram(&pldat->dma_buff_base_v, &dma_handle)) {
            pldat->dma_buff_base_v = NULL;
            pldat->dma_buff_size = 0;

Could this result in a zero-sized DMA allocation being used as an array
for descriptors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-dts_cleanup_arm_mcore-v1-0-0101795a2662@nxp.com?part=8