Re: [PATCH ath-next 11/17] wifi: ath12k: Add platform MSI support for CE

[Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>]

On 10/8/2026 4:11 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Resource leak of non-devm MSIs and IRQs on ath12k_ahb_probe() error paths
> --
> 
> commit ad41050e8b81bf8d1dd6e4fe6253604d235fd347
> Author: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>
> 
> wifi: ath12k: Add platform MSI support for CE
> 
> This commit adds support for configuring Copy Engine (CE) interrupts
> through platform MSI on hybrid AHB devices like the QCN6432. It introduces
> an IPCI MSI configuration, allocates MSI vectors from the platform MSI
> domain, and registers CE interrupt handlers using the allocated vectors.
> 
>> diff --git a/drivers/net/wireless/ath/ath12k/pcic.c b/drivers/net/wireless/ath/ath12k/pcic.c
>> --- a/drivers/net/wireless/ath/ath12k/pcic.c
>> +++ b/drivers/net/wireless/ath/ath12k/pcic.c
> [ ... ]
>> @@ -154,3 +155,157 @@ void ath12k_pcic_write32(struct ath12k_base *ab, u32 offset, u32 value)
> [ ... ]
>> +static int ath12k_pcic_ce_irq_config(struct ath12k_base *ab,
>> +				     struct msi_desc **msi_desc, int num_msi)
>> +{
> [ ... ]
>> +		scnprintf(ce_pipe->irq_name, sizeof(ce_pipe->irq_name),
>> +			  "pcic%u_wlan_ce_%u", userpd_id, i);
>> +		ret = request_irq(desc->irq, ath12k_pcic_ce_interrupt_handler,
>> +				  IRQF_SHARED, ce_pipe->irq_name, ce_pipe);
> 
> [Severity: High]
> Does this leak the requested IRQ on the probe error path?
> 
> Since request_irq() is used here instead of devm_request_irq(), it requires
> explicit cleanup. This function is called by ath12k_ahb_config_irq() during
> ath12k_ahb_probe(). If a subsequent initialization step in probe fails
> (such as arch_init or core_init), the error path jumps to labels like
> err_deinit_arch or err_core_free.
> 
> However, the error path in ath12k_ahb_probe() does not appear to call
> ath12k_ahb_disable_irq() or ath12k_pcic_free_irq() to free these IRQs.
> If the device context (ab) is subsequently freed by ath12k_core_free()
> while the interrupt handlers remain registered, could this lead to a
> use-after-free panic if an interrupt fires?
> 

Thanks for pointing this out. I'll address this in the next version.

>> +		if (ret) {
>> +			ath12k_warn(ab, "failed to request irq %d: %d\n", irq_idx, ret);
>> +			ath12k_pci_free_ce_irq(ab, i);
>> +			return ret;
>> +		}
> [ ... ]
>> +int ath12k_pcic_config_irq(struct ath12k_base *ab)
>> +{
>> +	int user_base_data, base_vector, num_vectors;
>> +	struct platform_device *pdev = ab->pdev;
>> +	int ret;
>> +
>> +	ab->msi.config = &ath12k_msi_config[ATH12K_MSI_CONFIG_IPCI];
>> +
>> +	ret = platform_device_msi_init_and_alloc_irqs(&pdev->dev,
>> +						      ab->msi.config->total_vectors,
>> +						      ath12k_msi_msg_handler);
> 
> [Severity: High]
> Similarly, does this platform MSI domain allocation get leaked on the
> ath12k_ahb_probe() error path?
> 
> platform_device_msi_init_and_alloc_irqs() is not devres managed, so the
> domain must be explicitly destroyed via platform_device_msi_free_irqs_all().
> Because ath12k_ahb_probe() does not call the corresponding free routine when
> it encounters an error after this point, are these MSI vectors leaked?
> 

Sure, I will address this in the next version.

>> +	if (ret)
>> +		return ret;
>