mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path
@ 2025-10-31 16:05 Bastien Curutchet (Schneider Electric)
  2025-10-31 16:05 ` [PATCH net 1/3] net: dsa: microchip: Fix checks on irq_find_mapping() Bastien Curutchet (Schneider Electric)
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Bastien Curutchet (Schneider Electric) @ 2025-10-31 16:05 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Richard Cochran
  Cc: Pascal Eberhard, Miquèl Raynal, Thomas Petazzoni, netdev,
	linux-kernel, Bastien Curutchet (Schneider Electric)

Hi all,

I worked on adding PTP support for the KSZ8463. While doing so, I ran
into a few bugs in the resource release process that occur when things go
wrong arount IRQ initialization.

This small series fixes those bugs.

The next series, which will add the PTP support, depend on this one.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
Bastien Curutchet (Schneider Electric) (3):
      net: dsa: microchip: Fix checks on irq_find_mapping()
      net: dsa: microchip: Ensure a ksz_irq is initialized before freeing it
      net: dsa: microchip: Immediately assing IRQ numbers

 drivers/net/dsa/microchip/ksz_common.c | 14 ++++++++------
 drivers/net/dsa/microchip/ksz_ptp.c    | 17 +++++++++--------
 2 files changed, 17 insertions(+), 14 deletions(-)
---
base-commit: cd2f741f5aec1043b707070e7ea024e646262277
change-id: 20251031-ksz-fix-db345df7635f

Best regards,
-- 
Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net 1/3] net: dsa: microchip: Fix checks on irq_find_mapping()
  2025-10-31 16:05 [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Bastien Curutchet (Schneider Electric)
@ 2025-10-31 16:05 ` Bastien Curutchet (Schneider Electric)
  2025-10-31 16:05 ` [PATCH net 2/3] net: dsa: microchip: Ensure a ksz_irq is initialized before freeing it Bastien Curutchet (Schneider Electric)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Bastien Curutchet (Schneider Electric) @ 2025-10-31 16:05 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Richard Cochran
  Cc: Pascal Eberhard, Miquèl Raynal, Thomas Petazzoni, netdev,
	linux-kernel, Bastien Curutchet (Schneider Electric)

irq_find_mapping() returns a positive IRQ number or 0 if no IRQ is found
but it never returns a negative value. However, on each
irq_find_mapping() call, we verify that the returned value isn't
negative.

Fix the irq_find_mapping() checks to enter error paths when 0 is
returned. Return -EINVAL in such cases.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 8 ++++----
 drivers/net/dsa/microchip/ksz_ptp.c    | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index a962055bfdbd8fbfc135b2dec73c222a213985c4..3a4516d32aa5f99109853ed400e64f8f7e2d8016 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2583,8 +2583,8 @@ static int ksz_irq_phy_setup(struct ksz_device *dev)
 
 			irq = irq_find_mapping(dev->ports[port].pirq.domain,
 					       PORT_SRC_PHY_INT);
-			if (irq < 0) {
-				ret = irq;
+			if (!irq) {
+				ret = -EINVAL;
 				goto out;
 			}
 			ds->user_mii_bus->irq[phy] = irq;
@@ -2948,8 +2948,8 @@ static int ksz_pirq_setup(struct ksz_device *dev, u8 p)
 	snprintf(pirq->name, sizeof(pirq->name), "port_irq-%d", p);
 
 	pirq->irq_num = irq_find_mapping(dev->girq.domain, p);
-	if (pirq->irq_num < 0)
-		return pirq->irq_num;
+	if (!pirq->irq_num)
+		return -EINVAL;
 
 	return ksz_irq_common_setup(dev, pirq);
 }
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 35fc21b1ee48a47daa278573bfe8749c7b42c731..c8bfbe5e2157323ecf29149d1907b77e689aa221 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1139,8 +1139,8 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 		irq_create_mapping(ptpirq->domain, irq);
 
 	ptpirq->irq_num = irq_find_mapping(port->pirq.domain, PORT_SRC_PTP_INT);
-	if (ptpirq->irq_num < 0) {
-		ret = ptpirq->irq_num;
+	if (!ptpirq->irq_num) {
+		ret = -EINVAL;
 		goto out;
 	}
 

-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net 2/3] net: dsa: microchip: Ensure a ksz_irq is initialized before freeing it
  2025-10-31 16:05 [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Bastien Curutchet (Schneider Electric)
  2025-10-31 16:05 ` [PATCH net 1/3] net: dsa: microchip: Fix checks on irq_find_mapping() Bastien Curutchet (Schneider Electric)
@ 2025-10-31 16:05 ` Bastien Curutchet (Schneider Electric)
  2025-10-31 16:05 ` [PATCH net 3/3] net: dsa: microchip: Immediately assing IRQ numbers Bastien Curutchet (Schneider Electric)
  2025-10-31 16:36 ` [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Maxime Chevallier
  3 siblings, 0 replies; 5+ messages in thread
From: Bastien Curutchet (Schneider Electric) @ 2025-10-31 16:05 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Richard Cochran
  Cc: Pascal Eberhard, Miquèl Raynal, Thomas Petazzoni, netdev,
	linux-kernel, Bastien Curutchet (Schneider Electric)

Sometimes ksz_irq_free() can be called on uninitialized (or partially
initialized) ksz_irq. It leads to freeing uninitialized IRQ numbers
and/or domains.

Ensure that IRQ numbers or domains are initialized before freeing them.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 3a4516d32aa5f99109853ed400e64f8f7e2d8016..4f5e2024442692adefc69d47e82381a3c3bda184 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2858,14 +2858,16 @@ static void ksz_irq_free(struct ksz_irq *kirq)
 {
 	int irq, virq;
 
-	free_irq(kirq->irq_num, kirq);
+	if (kirq->irq_num)
+		free_irq(kirq->irq_num, kirq);
 
 	for (irq = 0; irq < kirq->nirqs; irq++) {
 		virq = irq_find_mapping(kirq->domain, irq);
 		irq_dispose_mapping(virq);
 	}
 
-	irq_domain_remove(kirq->domain);
+	if (kirq->domain)
+		irq_domain_remove(kirq->domain);
 }
 
 static irqreturn_t ksz_irq_thread_fn(int irq, void *dev_id)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net 3/3] net: dsa: microchip: Immediately assing IRQ numbers
  2025-10-31 16:05 [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Bastien Curutchet (Schneider Electric)
  2025-10-31 16:05 ` [PATCH net 1/3] net: dsa: microchip: Fix checks on irq_find_mapping() Bastien Curutchet (Schneider Electric)
  2025-10-31 16:05 ` [PATCH net 2/3] net: dsa: microchip: Ensure a ksz_irq is initialized before freeing it Bastien Curutchet (Schneider Electric)
@ 2025-10-31 16:05 ` Bastien Curutchet (Schneider Electric)
  2025-10-31 16:36 ` [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Maxime Chevallier
  3 siblings, 0 replies; 5+ messages in thread
From: Bastien Curutchet (Schneider Electric) @ 2025-10-31 16:05 UTC (permalink / raw)
  To: Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Richard Cochran
  Cc: Pascal Eberhard, Miquèl Raynal, Thomas Petazzoni, netdev,
	linux-kernel, Bastien Curutchet (Schneider Electric)

The IRQ numbers created through irq_create_mapping() are only assigned
to ptpmsg_irq[n].num at the end of the IRQ setup. So if an error occurs
between their creation and their assignment (for instance during the
request_threaded_irq() step), we enter the error path and try to release
the not yet assigned ptpmsg_irq[n].num.

Assing the IRQ number at mapping creation.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index c8bfbe5e2157323ecf29149d1907b77e689aa221..a8ad99c6ee35ff60fb56cc5770520a793c86ff66 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1102,10 +1102,6 @@ static int ksz_ptp_msg_irq_setup(struct ksz_port *port, u8 n)
 
 	strscpy(ptpmsg_irq->name, name[n]);
 
-	ptpmsg_irq->num = irq_find_mapping(port->ptpirq.domain, n);
-	if (ptpmsg_irq->num < 0)
-		return ptpmsg_irq->num;
-
 	return request_threaded_irq(ptpmsg_irq->num, NULL,
 				    ksz_ptp_msg_thread_fn, IRQF_ONESHOT,
 				    ptpmsg_irq->name, ptpmsg_irq);
@@ -1135,8 +1131,13 @@ int ksz_ptp_irq_setup(struct dsa_switch *ds, u8 p)
 	if (!ptpirq->domain)
 		return -ENOMEM;
 
-	for (irq = 0; irq < ptpirq->nirqs; irq++)
-		irq_create_mapping(ptpirq->domain, irq);
+	for (irq = 0; irq < ptpirq->nirqs; irq++) {
+		port->ptpmsg_irq[irq].num = irq_create_mapping(ptpirq->domain, irq);
+		if (!port->ptpmsg_irq[irq].num) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
 
 	ptpirq->irq_num = irq_find_mapping(port->pirq.domain, PORT_SRC_PTP_INT);
 	if (!ptpirq->irq_num) {

-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path
  2025-10-31 16:05 [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Bastien Curutchet (Schneider Electric)
                   ` (2 preceding siblings ...)
  2025-10-31 16:05 ` [PATCH net 3/3] net: dsa: microchip: Immediately assing IRQ numbers Bastien Curutchet (Schneider Electric)
@ 2025-10-31 16:36 ` Maxime Chevallier
  3 siblings, 0 replies; 5+ messages in thread
From: Maxime Chevallier @ 2025-10-31 16:36 UTC (permalink / raw)
  To: Bastien Curutchet (Schneider Electric),
	Woojung Huh, UNGLinuxDriver, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Richard Cochran
  Cc: Pascal Eberhard, Miquèl Raynal, Thomas Petazzoni, netdev,
	linux-kernel

Hi Bastien,

On 31/10/2025 17:05, Bastien Curutchet (Schneider Electric) wrote:
> Hi all,
> 
> I worked on adding PTP support for the KSZ8463. While doing so, I ran
> into a few bugs in the resource release process that occur when things go
> wrong arount IRQ initialization.
> 
> This small series fixes those bugs.
> 
> The next series, which will add the PTP support, depend on this one.
> 

This series targets -net, however all the patches are missing a Fixes:
tag :(

You'll need to add them for the next revision :)

Maxime

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-10-31 16:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-31 16:05 [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Bastien Curutchet (Schneider Electric)
2025-10-31 16:05 ` [PATCH net 1/3] net: dsa: microchip: Fix checks on irq_find_mapping() Bastien Curutchet (Schneider Electric)
2025-10-31 16:05 ` [PATCH net 2/3] net: dsa: microchip: Ensure a ksz_irq is initialized before freeing it Bastien Curutchet (Schneider Electric)
2025-10-31 16:05 ` [PATCH net 3/3] net: dsa: microchip: Immediately assing IRQ numbers Bastien Curutchet (Schneider Electric)
2025-10-31 16:36 ` [PATCH net 0/3] net: dsa: microchip: Fix resource releases in error path Maxime Chevallier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®