Thread (7 messages) 7 messages, 2 authors, 4d ago

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

From: Jakub Raczynski <hidden>
Date: 2026-07-08 07:19:02
Also in: linux-arm-kernel, lkml

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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help