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

[Jakub Raczynski <j.raczynski@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]

On Tue, Jul 07, 2026 at 08:57:28PM +0200, Andrew Lunn wrote:
> 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?

Yes, more below

> 
> >  	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
>

EINVAL would be better, I agree.

Mentioned patch changed U8_MAX -> MTL_MAX_RX_QUEUES (and TX), because
inputing more just panicked kernel and there was already verification for
that, so it was fixed by silent fix of incorrect value.
We triggered this while were testing XGMAC hardware that supports 16 queues
(current driver limit is 8). Original fix having EINVAL would also be good.

Inputing 0 is much less honest mistake but rather some testing how kernel
handles other/incorrect values. So at the very least, this should not be able
to panic kernel. But as above, I agree EINVAL actually would fit better here,
that will be for V2, and so it would for original 8a7bca6de6de.

Whether it justifies Fixes or not, I am not sure, it is real possibility to
trigger panic if system has dynamic DTS loading (CONFIG_OF_OVERLAY) on
system with wrong permissions.

BR
Jakub Raczynski

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]