If ksz_pirq_setup() fails after at least one successful port
initialization, the goto jumps directly to the global irq freeing,
leaking the resources of the previously initialized ports.

Fix the goto jump to release all the potentially initialized ports.
Remove the no-longer used out_girq label.

Fixes: c9cd961c0d43 ("net: dsa: microchip: lan937x: add interrupt support for port phy link")
Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index a622416d966330187ee062b2f44051ddf4ce2a78..2b6f7abea00776fafff0c1774cab297a7ef261da 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3035,7 +3035,7 @@ static int ksz_setup(struct dsa_switch *ds)
 		dsa_switch_for_each_user_port(dp, dev->ds) {
 			ret = ksz_pirq_setup(dev, dp->index);
 			if (ret)
-				goto out_girq;
+				goto out_pirq;
 
 			if (dev->info->ptp_capable) {
 				ret = ksz_ptp_irq_setup(ds, dp->index);
@@ -3083,10 +3083,8 @@ static int ksz_setup(struct dsa_switch *ds)
 			if (dev->ports[dp->index].pirq.domain)
 				ksz_irq_free(&dev->ports[dp->index].pirq);
 		}
-	}
-out_girq:
-	if (dev->irq > 0)
 		ksz_irq_free(&dev->girq);
+	}
 
 	return ret;
 }

-- 
2.51.1

Hi Bastien,

On 17/11/2025 14:05, Bastien Curutchet (Schneider Electric) wrote:
> If ksz_pirq_setup() fails after at least one successful port
> initialization, the goto jumps directly to the global irq freeing,
> leaking the resources of the previously initialized ports.
> 
> Fix the goto jump to release all the potentially initialized ports.
> Remove the no-longer used out_girq label.
> 
> Fixes: c9cd961c0d43 ("net: dsa: microchip: lan937x: add interrupt support for port phy link")
> Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
> ---
>  drivers/net/dsa/microchip/ksz_common.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index a622416d966330187ee062b2f44051ddf4ce2a78..2b6f7abea00776fafff0c1774cab297a7ef261da 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -3035,7 +3035,7 @@ static int ksz_setup(struct dsa_switch *ds)
>  		dsa_switch_for_each_user_port(dp, dev->ds) {
>  			ret = ksz_pirq_setup(dev, dp->index);
>  			if (ret)
> -				goto out_girq;
> +				goto out_pirq;
>  
>  			if (dev->info->ptp_capable) {
>  				ret = ksz_ptp_irq_setup(ds, dp->index);
> @@ -3083,10 +3083,8 @@ static int ksz_setup(struct dsa_switch *ds)
>  			if (dev->ports[dp->index].pirq.domain)
>  				ksz_irq_free(&dev->ports[dp->index].pirq);
>  		}
> -	}
> -out_girq:
> -	if (dev->irq > 0)
>  		ksz_irq_free(&dev->girq);
> +	}
>  
>  	return ret;
>  }
> 

Looking at the code, I think it's still not enough, but feel free
to correct me.

In ksz_setup(), in one single loop we do :

dsa_switch_for_each_user_port(dp, dev->ds) {
	ksz_pirq_setup();

	ksz_ptp_irq_setup();
}

However when anything fails in the above loop, we jump straight
to out_pirq, which doesn't clean the ptpirq :(

Maxime

Hi Maxime,

On 11/17/25 5:28 PM, Maxime Chevallier wrote:
> Hi Bastien,
> 
> On 17/11/2025 14:05, Bastien Curutchet (Schneider Electric) wrote:
>> If ksz_pirq_setup() fails after at least one successful port
>> initialization, the goto jumps directly to the global irq freeing,
>> leaking the resources of the previously initialized ports.
>>
>> Fix the goto jump to release all the potentially initialized ports.
>> Remove the no-longer used out_girq label.
>>
>> Fixes: c9cd961c0d43 ("net: dsa: microchip: lan937x: add interrupt support for port phy link")
>> Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
>> ---
>>   drivers/net/dsa/microchip/ksz_common.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
>> index a622416d966330187ee062b2f44051ddf4ce2a78..2b6f7abea00776fafff0c1774cab297a7ef261da 100644
>> --- a/drivers/net/dsa/microchip/ksz_common.c
>> +++ b/drivers/net/dsa/microchip/ksz_common.c
>> @@ -3035,7 +3035,7 @@ static int ksz_setup(struct dsa_switch *ds)
>>   		dsa_switch_for_each_user_port(dp, dev->ds) {
>>   			ret = ksz_pirq_setup(dev, dp->index);
>>   			if (ret)
>> -				goto out_girq;
>> +				goto out_pirq;
>>   
>>   			if (dev->info->ptp_capable) {
>>   				ret = ksz_ptp_irq_setup(ds, dp->index);
>> @@ -3083,10 +3083,8 @@ static int ksz_setup(struct dsa_switch *ds)
>>   			if (dev->ports[dp->index].pirq.domain)
>>   				ksz_irq_free(&dev->ports[dp->index].pirq);
>>   		}
>> -	}
>> -out_girq:
>> -	if (dev->irq > 0)
>>   		ksz_irq_free(&dev->girq);
>> +	}
>>   
>>   	return ret;
>>   }
>>
> 
> Looking at the code, I think it's still not enough, but feel free
> to correct me.
> 
> In ksz_setup(), in one single loop we do :
> 
> dsa_switch_for_each_user_port(dp, dev->ds) {
> 	ksz_pirq_setup();
> 
> 	ksz_ptp_irq_setup();
> }
> 
> However when anything fails in the above loop, we jump straight
> to out_pirq, which doesn't clean the ptpirq :(

Good catch, I should also merge together the out_ptpirq and out_pirq 
labels then. And ensure that the PTP IRQ is initialized before freeing it.

I'll respin with that.


Best regards,
Bastien