Re: [PATCH net-next v9 4/4] net: stmmac: Add BCM8958x driver to build system

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: stmmac: Add BCM8958x driver to build system

This commit adds the PCI driver for the Broadcom BCM8958x to the Linux build
system. It enables the configuration option and updates the MAINTAINERS file.

While reviewing this patch, I noticed a potential regression introduced in the
previous patch of the series (net: stmmac: Add PCI glue driver for BCM8958x)
that is enabled here.

The driver defines a global static array of software nodes (brcm_swnodes)
and registers it in brcm_drv_init() using software_node_register_node_group().

Since PCI drivers must support multiple device instances concurrently, what
happens if there are multiple BCM8958x PCI devices present?

If the second instance's probe attempts to register the exact same software
node pointers, software_node_register() will return -EEXIST. The failure
path in software_node_register_node_group() then unconditionally unregisters
all nodes in the group.

Could this unregister the software nodes that the first probed device
successfully registered and is actively using, leaving the first device with
dangling fwnode pointers and leading to a use-after-free? Would it be safer
to dynamically allocate the software nodes per device instance?

[ ... ]

> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> index dc55aec915a27..a1e39f65cd42e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -51,4 +51,5 @@ 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_DWMAC_BRCM)       += dwmac-brcm.o

This isn't a bug, but it looks like spaces are used here for alignment instead
of a tab. The surrounding entries all use a tab character before the
assignment operator.