Re: [PATCH v5 09/13] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential deadlock because `cxl_cper_prot_err_work_lock` is acquired in process context without disabling hardware interrupts.
--

commit 1416c4f9e4b0f4dc84b6e812f70c5f34a5255acc
Author: Dave Jiang <dave.jiang@intel.com>

ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy

cxl_cper_setup_prot_err_work_data() locates the RAS Capability block using an
unvalidated firmware-controlled length, which can point the copy outside the
section. This commit extends cxl_cper_sec_prot_err_valid() to check that the
section can hold the header, DVSEC, and RAS Capability block within the reported
section length.

Link: https://sashiko.dev/#/patchset/20260617-topics-ahmtib01-ras_ffh_arm_internal_review-v6-0-91f725174aa0@arm.com?part=6
Link: https://lore.kernel.org/linux-cxl/20260709165457.8BA181F000E9@smtp.kernel.org/

[ ... ]

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index e5f8dbd17017e..b8dbd99da47ef 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -753,12 +753,12 @@ static DEFINE_SPINLOCK(cxl_cper_prot_err_work_lock);
>  struct work_struct *cxl_cper_prot_err_work;
>  
>  void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> -			    int severity)
> +			    int severity, u32 len)
>  {
>  #ifdef CONFIG_ACPI_APEI_PCIEAER
>  	struct cxl_cper_prot_err_work_data wd;
>  
> -	if (cxl_cper_sec_prot_err_valid(prot_err))
> +	if (cxl_cper_sec_prot_err_valid(prot_err, len))
>  		return;
>  
>  	guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);

[Severity: High]
This isn't a bug introduced by this patch, but does acquiring this lock in
hardirq context cause a potential deadlock?

If the module initialization thread executes cxl_cper_register_prot_err_work()
(or unregister) in process context, it acquires cxl_cper_prot_err_work_lock
with interrupts enabled:

drivers/acpi/apei/ghes.c:cxl_cper_register_prot_err_work() {
    ...
	guard(spinlock)(&cxl_cper_prot_err_work_lock);
    ...
}

If a GHES hardware error interrupt fires on the same CPU, executing
ghes_do_proc() -> cxl_cper_post_prot_err(), it will attempt to acquire the
same lock here using guard(spinlock_irqsave).

Would it spin forever waiting for the interrupted process context to release
the lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827203726.3027541-1-dave.jiang@intel.com?part=9