Re: [PATCH v8 net-next 10/15] net: dsa: netc: introduce NXP NETC switch driver for i.MX94

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using device-managed memory allocations (`devm_`) and registrations inside the DSA `setup` callback causes memory leaks and re-registration failures.
- [High] The `netc_switch_shutdown` handler does not disable device DMA, which can lead to memory corruption during kexec.
--

commit 9bab65d8563212abf27d7fd39b5e794000f38524
Author: Wei Fang <wei.fang@nxp.com>

net: dsa: netc: introduce NXP NETC switch driver for i.MX94

This commit introduces the NXP NETC switch driver for the i.MX94 series,
integrating an 802.1Q Ethernet switch with the internal ENETC via a pseudo
MAC. It provides basic probe and remove functionality.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> --- /dev/null
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> +static int netc_port_create_emdio_bus(struct netc_port *np,
> +				      struct device_node *node)
> +{
[ ... ]
> +	bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
> +	if (!bus)
> +		return -ENOMEM;
[ ... ]
> +	err = devm_of_mdiobus_register(dev, bus, node);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "Cannot register EMDIO bus\n");
> +
> +	np->emdio = bus;
> +
> +	return 0;
> +}
[ ... ]
> +static int netc_init_all_ports(struct netc_switch *priv)
> +{
[ ... ]
> +	priv->ports = devm_kcalloc(dev, priv->info->num_ports,
> +				   sizeof(struct netc_port *),
> +				   GFP_KERNEL);
> +	if (!priv->ports)
> +		return -ENOMEM;
> +
> +	/* Some DSA interfaces may set the port even it is disabled, such
> +	 * as .port_disable(), .port_stp_state_set() and so on. To avoid
> +	 * crash caused by accessing NULL port pointer, each port is
> +	 * allocated its own memory. Otherwise, we need to check whether
> +	 * the port pointer is NULL in these interfaces. The latter is
> +	 * difficult for us to cover.
> +	 */
> +	for (int i = 0; i < priv->info->num_ports; i++) {
> +		np = devm_kzalloc(dev, sizeof(*np), GFP_KERNEL);
> +		if (!np)
> +			return -ENOMEM;
> +
> +		np->switch_priv = priv;
> +		np->iobase = priv->regs.port + PORT_IOBASE(i);
> +		netc_port_get_capability(np);
> +		priv->ports[i] = np;
> +	}

Are these device-managed allocations safe inside the DSA setup path?

The DSA subsystem allows the switch tree to be torn down and set up multiple
times during the device's lifetime, such as when the conduit network interface
is unbound and re-bound.

Because devm_ resources are tied to the PCI device's lifecycle rather than
the DSA switch tree lifecycle, they are not freed during teardown().

Will subsequent calls to setup() leak memory and fail to register the MDIO
bus due to naming collisions?

[ ... ]
> +static int netc_init_all_cbdrs(struct netc_switch *priv)
> +{
> +	struct netc_switch_regs *regs = &priv->regs;
> +	struct ntmp_user *ntmp = &priv->ntmp;
> +	int i, err;
> +
> +	ntmp->cbdr_num = NETC_CBDR_NUM;
> +	ntmp->dev = priv->dev;
> +	ntmp->ring = devm_kcalloc(ntmp->dev, ntmp->cbdr_num,
> +				  sizeof(struct netc_cbdr),
> +				  GFP_KERNEL);

Similarly, will this device-managed allocation leak memory when the switch
is repeatedly set up?

[ ... ]
> +static void netc_switch_shutdown(struct pci_dev *pdev)
> +{
> +	struct netc_switch *priv = pci_get_drvdata(pdev);
> +
> +	if (!priv)
> +		return;
> +
> +	dsa_switch_shutdown(priv->ds);
> +	pci_set_drvdata(pdev, NULL);
> +}

Does this shutdown handler leave DMA active?

The driver enables Bus Master DMA in netc_switch_pci_init() via
pci_set_master(), but this function only brings down the network interfaces
without clearing the Bus Master Enable bit (e.g., via pci_clear_master()).

Could this cause silent memory corruption during a kexec, if the active PCI
hardware continues transferring data into memory regions that are now owned
by the new kernel?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518082506.1318236-1-wei.fang@nxp.com?part=10