Re: [PATCH net-next v2 13/14] net: stmmac: tc956x: add TC956x/QPS615 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Leaked IRQ mappings and potential Use-After-Free on the probe error path if IRQ mapping or resource initialization fails.
- [High] Uninitialized stack variables passed to `devm_irq_domain_instantiate()` can cause a kernel panic or memory corruption.
- [Low] Invalid Makefile rule referencing a non-existent Kconfig symbol and missing file.
--

commit 55f951da9fbefea49a26410638dbb60af63d2658
Author: Daniel Thompson <daniel@riscstar.com>

net: stmmac: tc956x: add TC956x/QPS615 support

Toshiba TC956x is an Ethernet AVB/TSN bridge and is essentially a small
and highly-specialized SoC. This commit adds a driver for the eMAC/MSIGEN
combination found on these devices.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index a1cea2f57252e..e8e7f95dbe3e8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -51,4 +51,6 @@ obj-$(CONFIG_STMMAC_PCI)	+= stmmac-pci.o
>  obj-$(CONFIG_DWMAC_INTEL)	+= dwmac-intel.o
>  obj-$(CONFIG_DWMAC_LOONGSON)	+= dwmac-loongson.o
>  obj-$(CONFIG_DWMAC_MOTORCOMM)	+= dwmac-motorcomm.o
> +obj-$(CONFIG_TC956X_PCI)	+= tc956x-pci.o

[Severity: Low]
Is this line a leftover from a previous iteration? The correct Kconfig
symbol appears to be CONFIG_TOSHIBA_TC956X_PCI, and the source file
tc956x_pci.c was placed in drivers/misc/ rather than the stmmac directory.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-tc956x.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-tc956x.c
> new file mode 100644
> index 0000000000000..c77585e4a50e6
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-tc956x.c
[ ... ]
> +/* We have one IRQ chip instance with 25 IRQs in its domain */
> +static struct irq_domain *
> +tc956x_msigen_irq_domain_instantiate(struct tc956x_data *td)
> +{
> +	struct irq_domain_chip_generic_info dgc_info;
> +	struct irq_domain_info info;

[Severity: High]
Does this pass uninitialized stack variables to the core IRQ domain subsystem?

These local variables are declared on the stack without zero-initialization.
Since only a subset of their fields are populated before passing them to
devm_irq_domain_instantiate(), fields like info.fwnode and info.parent could
contain arbitrary stack garbage.

If the core IRQ domain subsystem tries to dereference a garbage pointer in
info.fwnode, could it lead to memory corruption or a page fault during probe?

> +
> +	dgc_info.name = devm_kasprintf(td->dev, GFP_KERNEL, "tc956x-msigen-%d",
> +				       td->auxbus_data->mac_id);
[ ... ]
> +	info.init = tc956x_msigen_irq_domain_init;
> +	info.exit = tc956x_msigen_irq_domain_exit;
> +
> +	return devm_irq_domain_instantiate(td->dev, &info);
> +}
[ ... ]
> +static int tc956x_stmmac_resources_init(struct tc956x_data *td)
> +{
> +	struct irq_domain *irq_domain = td->irq_domain;
> +	struct stmmac_resources *res = &td->res;
> +	u32 i;
> +
> +	res->irq = irq_create_mapping(irq_domain, HWIRQ_EVENT);
> +	if (!res->irq)
> +		return -EINVAL;
> +
> +	for (i = 0; i < td->plat->tx_queues_to_use; i++) {
> +		res->tx_irq[i] = irq_create_mapping(irq_domain, HWIRQ_TX0 + i);
> +		if (!res->tx_irq[i])
> +			return -EINVAL;

[Severity: High]
If irq_create_mapping() fails here or in the RX loop below, this returns
an error without unwinding previously successful mappings. 

When the caller tc956x_dwmac_probe() encounters this error:

tc956x_dwmac_probe() {
    ...
	ret = tc956x_stmmac_resources_init(td);
	if (ret) {
		ret = dev_err_probe(dev, ret,
				    "failed to initialize stmmac resources\n");
		goto err_put_mdio;
	}
    ...
}

It jumps to err_put_mdio, bypassing tc956x_stmmac_resources_exit() entirely.

Since devres will automatically free the irq_domain on probe failure, will the
undisposed mappings cause a kernel warning and leave undisposed irq_desc
structures pointing to the freed domain, leading to a use-after-free?

> +	}
> +
> +	for (i = 0; i < td->plat->rx_queues_to_use; i++) {
> +		res->rx_irq[i] = irq_create_mapping(irq_domain, HWIRQ_RX0 + i);
> +		if (!res->rx_irq[i])
> +			return -EINVAL;
> +	}
> +
> +	res->addr = td->auxbus_data->emac;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605010022.968612-1-elder@riscstar.com?part=13