[PATCH net 0/2] net/stmmac: Verify more wrong DTS configuration

[PATCH net 0/2] net/stmmac: Verify more wrong DTS configuration

[Jakub Raczynski]

Commit 8a7bca6de6de protected against possible wrong number of TX/RX queues,
which could happen with new XGMAC hardware that has more supported queues than
currently available in driver.
Sashiko mentioned that there should also be protection against zero input
and some AXI related config, as these are possible kernel panics too.
While this series has lower value than original patch, since this
misconfiguration should not happen by pure mistake, there is no reason not
to fix it.

Jakub Raczynski (2):
  net/stmmac: Protect against zero queue DTS config
  net/stmmac: Verify provided DTS AXI setup

 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH net 1/2] net/stmmac: Protect against zero queue DTS config

[Jakub Raczynski]

Commit 8a7bca6de6de protected against inputing number of tx/rx_queues_to_use
over kernel supported limit in DTS config. AI review mentioned that we also
should protect against zero queue input, because this would cause issues
down the line. Missing config is not an issue as stmmac_plat_dat_alloc()
does apply '1' by default.

Fix this by adding check for zero queues input during DTS parsing

Fixes: 8a7bca6de6de ("net/stmmac: Apply MTL_MAX queue limit if config missing")
Reported-by: Sashiko AI <sashiko-bot@kernel.org>
Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index dc5f951a311d..9112cd69b9b1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -158,6 +158,8 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
 	if (!of_property_read_u32(rx_node, "snps,rx-queues-to-use", &value)) {
 		if (value > MTL_MAX_RX_QUEUES)
 			value = MTL_MAX_RX_QUEUES;
+		else if (value == 0)
+			value = 1;
 		plat->rx_queues_to_use = value;
 	}
 
@@ -212,6 +214,8 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
 	if (!of_property_read_u32(tx_node, "snps,tx-queues-to-use", &value)) {
 		if (value > MTL_MAX_TX_QUEUES)
 			value = MTL_MAX_TX_QUEUES;
+		else if (value == 0)
+			value = 1;
 		plat->tx_queues_to_use = value;
 	}
 
-- 
2.34.1

Re: [PATCH net 1/2] net/stmmac: Protect against zero queue DTS config

[Andrew Lunn]

On Tue, Jul 07, 2026 at 07:44:30PM +0200, Jakub Raczynski wrote:
> Commit 8a7bca6de6de protected against inputing number of tx/rx_queues_to_use
> over kernel supported limit in DTS config. AI review mentioned that we also
> should protect against zero queue input, because this would cause issues
> down the line. Missing config is not an issue as stmmac_plat_dat_alloc()
> does apply '1' by default.
> 
> Fix this by adding check for zero queues input during DTS parsing
> 
> Fixes: 8a7bca6de6de ("net/stmmac: Apply MTL_MAX queue limit if config missing")

I'm not sure a Fixes: is justified here. Does this bother somebody? As
far as i understand, for this to actually do something the system is
broken anyway?

>  	if (!of_property_read_u32(rx_node, "snps,rx-queues-to-use", &value)) {
>  		if (value > MTL_MAX_RX_QUEUES)
>  			value = MTL_MAX_RX_QUEUES;
> +		else if (value == 0)
> +			value = 1;

If the DT is broken, don't we want it to be fixed? -EINVAL would make
it obvious.

    Andrew

---
pw-bot: cr

[PATCH net 2/2] net/stmmac: Verify provided DTS AXI setup

[Jakub Raczynski]

During parsing of AXI setup, there are few issues:
- 'axi_blen' array is uninitialized value on stack without zero-init stack
  configured. This can result in random AXI burst length config if
  DTS config provides shorter array than AXI_BLEN.
  Fix this by initializing it to zero, which allows to provide shorter configs,
  which sometimes happens where only one value is used.

- stmmac_axi_blen_to_mask() is executed regardless of return of
  of_property_read_u32_array(). This is not issue after axi_blen is
  initialized to zero, as zero blen values are skipped in
  stmmac_axi_blen_to_mask(), but that would be useless operation.
  Fix it by checking return value of of_property_read_u32_array() and
  parse any legit config.

- In case of failed memory allocation for AXI and error, there is no handling
  of that. Fix it by checking if AXI config is error and return if so,
  as this can only lack of memory. No AXI config, although is probably
  wrong in most cases, is not treated as error, as generic config is mostly
  provided in drivers.

Fixes: afea03656add ("stmmac: rework DMA bus setting and introduce new platform AXI structure")
Reported-by: Sashiko AI <sashiko-bot@kernel.org>
Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 9112cd69b9b1..0acc61a98292 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -95,7 +95,7 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
 {
 	struct device_node *np;
 	struct stmmac_axi *axi;
-	u32 axi_blen[AXI_BLEN];
+	u32 axi_blen[AXI_BLEN] = { 0 };
 
 	np = of_parse_phandle(pdev->dev.of_node, "snps,axi-config", 0);
 	if (!np)
@@ -115,8 +115,8 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)
 		axi->axi_wr_osr_lmt = 1;
 	if (of_property_read_u32(np, "snps,rd_osr_lmt", &axi->axi_rd_osr_lmt))
 		axi->axi_rd_osr_lmt = 1;
-	of_property_read_u32_array(np, "snps,blen", axi_blen, AXI_BLEN);
-	stmmac_axi_blen_to_mask(&axi->axi_blen_regval, axi_blen, AXI_BLEN);
+	if (!of_property_read_u32_array(np, "snps,blen", axi_blen, AXI_BLEN))
+		stmmac_axi_blen_to_mask(&axi->axi_blen_regval, axi_blen, AXI_BLEN);
 	of_node_put(np);
 
 	return axi;
@@ -580,6 +580,10 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 	of_property_read_u32(np, "snps,ps-speed", &plat->mac_port_sel_speed);
 
 	plat->axi = stmmac_axi_setup(pdev);
+	if (IS_ERR(plat->axi)) {
+		ret = plat->axi;
+		goto error_put_mdio;
+	}
 
 	rc = stmmac_mtl_setup(pdev, plat);
 	if (rc) {
-- 
2.34.1

Re: [PATCH net 2/2] net/stmmac: Verify provided DTS AXI setup

[Andrew Lunn]

On Tue, Jul 07, 2026 at 07:44:31PM +0200, Jakub Raczynski wrote:
> During parsing of AXI setup, there are few issues:
> - 'axi_blen' array is uninitialized value on stack without zero-init stack
>   configured. This can result in random AXI burst length config if
>   DTS config provides shorter array than AXI_BLEN.

What does the DT blinding say about the length? Is it allowed to be
short?

Are we talking about:

      snps,blen:
        $ref: /schemas/types.yaml#/definitions/uint32-array
        description:
          this is a vector of supported burst length.
        minItems: 7
        maxItems: 7

So it should be 7. Are there any in kernel DT blobs which don't pass
7? Can we just error out when it is not 7?

> - In case of failed memory allocation for AXI and error, there is no handling
>   of that. Fix it by checking if AXI config is error and return if so,
>   as this can only lack of memory. No AXI config, although is probably
>   wrong in most cases, is not treated as error, as generic config is mostly
>   provided in drivers.

This seems like a different fix. Maybe put it into a patch of its own.

> Fixes: afea03656add ("stmmac: rework DMA bus setting and introduce new platform AXI structure")

Again, does this bother anybody? At least axi_blen issue seems to be
that the DT blob is broken, so i doubt it actually does. Are there
reports of memory allocation error and resulting Opps.


    Andrew

---
pw-bot: cr