netdev - Re: [PATCH net 1/2] net: stmmac: Limit FIFO size by hardware feature value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c54a6ef-83ab-4739-bf2e-414c4d4621dc@lunn.ch>
Date: Mon, 20 Jan 2025 17:29:01 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <joabreu@...opsys.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 1/2] net: stmmac: Limit FIFO size by hardware feature
 value

On Mon, Jan 20, 2025 at 02:20:23PM +0900, Kunihiko Hayashi wrote:
> Hi Andrew,
> 
> On 2025/01/17 5:16, Andrew Lunn wrote:
> > On Thu, Jan 16, 2025 at 11:08:52AM +0900, Kunihiko Hayashi wrote:
> > > Tx/Rx FIFO size is specified by the parameter "{tx,rx}-fifo-depth" from
> > > the platform layer.
> > > 
> > > However, these values are constrained by upper limits determined by the
> > > capabilities of each hardware feature. There is a risk that the upper
> > > bits will be truncated due to the calculation, so it's appropriate to
> > > limit them to the upper limit values.
> > 
> > Are these values hard coded in the platform layer? Or can they come
> > from userspace?
> 
> My explanation is insufficient and misleading.
> "From the platform layer" means the common layer of stmmac described in
> "stmmac_platform.c".
> 
> > If they are hard coded, we should also fix them. So maybe add a
> > netdev_warn(), and encourage the platform maintainers to fix their
> > platform. If they are coming from userspace, we should consider
> > failing the ethtool call with an -EINVAL, and maybe an extack with the
> > valid range?
> 
> These values are derived from the devicetree and stored in the stmmac
> private structure. They are hardware-specific values, so I think this
> fix is sufficient.

But if they are coming from device tree, the device tree developer has
made an error, which has been silently ignored. Do we want to leave
the device tree broken? Or should we encourage developers to fix them?
Printing a warning would facilitate that.

	Andrew