linux-kernel - Re: [PATCH net v3 3/4] net: dsa: microchip: Ensure a ksz

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] [day] [month] [year] [list]

Message-ID: <f13d7a80-d7cc-46c4-99f4-ea42f419b252@lunn.ch>
Date: Fri, 14 Nov 2025 16:00:21 +0100
From: Andrew Lunn <andrew@...n.ch>
To: "Bastien Curutchet (Schneider Electric)" <bastien.curutchet@...tlin.com>
Cc: Woojung Huh <woojung.huh@...rochip.com>, UNGLinuxDriver@...rochip.com,
	Vladimir Oltean <olteanv@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Richard Cochran <richardcochran@...il.com>,
	Arun Ramadoss <arun.ramadoss@...rochip.com>,
	Pascal Eberhard <pascal.eberhard@...com>,
	Miquèl Raynal <miquel.raynal@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v3 3/4] net: dsa: microchip: Ensure a ksz_irq is
 initialized before freeing it

On Fri, Nov 14, 2025 at 08:20:22AM +0100, Bastien Curutchet (Schneider Electric) wrote:
> Sometimes ksz_irq_free() can be called on uninitialized ksz_irq (for
> example when ksz_ptp_irq_setup() fails). It leads to freeing
> uninitialized IRQ numbers and/or domains.
> 
> Ensure that IRQ numbers or domains aren't null before freeing them.
> In our case the IRQ number of an initialized ksz_irq is never 0. Indeed,
> it's either the device's IRQ number and we enter the IRQ setup only when
> this dev->irq is strictly positive, or a virtual IRQ assigned with
> irq_create_mapping() which returns strictly positive IRQ numbers.
> 
> Fixes: cc13ab18b201 ("net: dsa: microchip: ptp: enable interrupt for timestamping")
> Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@...tlin.com>
> --
> Regarding the Fixes tag here, IMO before cc13ab18b201 it was safe to
> not check the domain and the IRQ number because I don't see any path
> where ksz_irq_free() would be called on a non-initialized ksz_irq

I would say the caller is wrong, not ksz_irq_free().

Functions like this come in pairs: ksz_irq_setup() & ksz_irq_free().
_free() should not be called if _setup() was not successful.

Please take a look if you can fix the caller. If the change is big,
maybe we need this as a minimal fix for net, but make the bigger
change in net-next?

Thanks
	Andrew