Re: [PATCH 4/4] arm64: dts: qcom: shikra: add LCP/SRP ECC interrupt to LLCC node

[Faiyaz Mohammed <faiyaz.mohammed@oss.qualcomm.com>]

On 14-10-2026 21:38, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] edac: qcom: UAF in IRQ handlers due to asymmetrical devm vs manual teardown
This is a DTS-only patch; the driver concern is addressed in the
response to Patch 3.

The identical race exists for the pre-existing ecc_irq INT path which
also uses devm_request_irq() with edev_ctl as dev_id. Fixing only
lcp_irq would create an asymmetry. The correct fix is to convert both
IRQs to non-devm with explicit free_irq() before
edac_device_free_ctl_info() in qcom_llcc_edac_remove() — that is a
pre-existing driver-wide cleanup not introduced by this series.

> --
> 
> commit fab541f0482e82717fee45392a6aa89253bbeef9
> Author: Faiyaz Mohammed <faiyaz.mohammed@oss.qualcomm.com>
> 
> arm64: dts: qcom: shikra: add LCP/SRP ECC interrupt to LLCC node
> 
> Add a second interrupt for LCP/SRP DDR DRAM ECC reporting and
> interrupt-names ("llcc-ecc", "llcc-lcp-ecc") to the Shikra LLCC node,
> as required by the updated DT binding.
> 
>> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
>> index 2623e139196f6..ff572c003b396 100644
>> --- a/arch/arm64/boot/dts/qcom/shikra.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
>> @@ -1108,7 +1108,9 @@ llcc: system-cache-controller@e00000 {
>>  			reg-names = "llcc0_base",
>>  				    "llcc1_base",
>>  				    "llcc_broadcast_base";
>> -			interrupts = <GIC_SPI 539 IRQ_TYPE_LEVEL_HIGH 0>;
>> +			interrupts = <GIC_SPI 539 IRQ_TYPE_LEVEL_HIGH 0>,
>> +				     <GIC_SPI 538 IRQ_TYPE_LEVEL_HIGH 0>;
>> +			interrupt-names = "llcc-ecc", "llcc-lcp-ecc";
> 
> [Severity: High]
> Does adding this new interrupt expose a use-after-free race condition in the
> EDAC driver?
> 
> In drivers/edac/qcom_edac.c, devm-managed IRQs remain active until the devres
> phase, but edac_device_free_ctl_info() manually frees the payload object:
> 
> drivers/edac/qcom_edac.c:qcom_llcc_edac_remove() {
> 	...
> 	edac_device_free_ctl_info(edev_ctl);
> 	...
> }
> 
> If the newly enabled lcp_irq or the ecc_irq fires during module removal
> (after qcom_llcc_edac_remove() frees edev_ctl but before devres deregisters
> the IRQs), can the interrupt handler dereference the freed edev_ctl?
> 
> drivers/edac/qcom_edac.c:llcc_lcp_ecc_irq_handler() {
> 	...
> 	struct llcc_drv_data *drv = edac_dev_ctl->dev->platform_data;
> 	...
> }
> 
> This could lead to invalid memory accesses, unpredictable behavior,
> or a kernel panic.
> 
> The same race seems to exist on the probe error path if
> edac_device_add_device() fails, leaving the IRQ active until the function
> returns:
> 
> drivers/edac/qcom_edac.c:qcom_llcc_edac_probe() {
> 	...
> 	rc = edac_device_add_device(edev_ctl);
> 	if (rc) {
> 		edac_device_free_ctl_info(edev_ctl);
> 		return rc;
> 	}
> 	...
> }
> 
>>  		};
>>  
>>  		gcc: clock-controller@1400000 {
> 

Thanks & regards,
Mohammed Faiyaz