Re: [PATCH v19 03/14] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds checks on CPER CXL event and protocol error payloads can cause out-of-bounds reads, potentially leaking kernel memory via trace events or crashing the kernel.
--

--- Patch [3]: [PATCH v19 03/14] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks ---
commit 9ad943c529c38ff6736192a1064f09367d33c360
Author: Terry Bowman <terry.bowman@amd.com>

acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks

The CXL CPER work registration and unregistration helpers acquire
cxl_cper_work_lock and cxl_cper_prot_err_work_lock with a spinlock
guard(), which leaves local interrupts enabled. This commit converts
both locks from spinlock_t to raw_spinlock_t to prevent deadlocks with
the GHES error notification path executing in hard IRQ context.

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 3236a3ce79d6b..ca7a138c1ff2e 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
[ ... ]
> @@ -761,7 +761,7 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
>  	if (cxl_cper_sec_prot_err_valid(prot_err))
>  		return;
>  
> -	guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
> +	guard(raw_spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
>  
>  	if (!cxl_cper_prot_err_work)
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for this code
to read out of bounds?

In cxl_cper_post_prot_err(), it subsequently calls
cxl_cper_setup_prot_err_work_data() (in ghes_helpers.c) which does the
following:

	memcpy(&wd->prot_err, prot_err, sizeof(wd->prot_err));

	dvsec_start = (u8 *)(prot_err + 1);
	cap_start = dvsec_start + prot_err->dvsec_len;

	memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap));

Could an untrusted or overly large dvsec_len cause cap_start to point
to unmapped memory, resulting in a page fault during the memcpy?

If it reads adjacent mapped memory, could that out-of-bounds data be leaked
to userspace via trace events when the workqueue processes the struct?

[ ... ]
> @@ -831,7 +839,7 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
>  		return;
>  	}
>  
> -	guard(spinlock_irqsave)(&cxl_cper_work_lock);
> +	guard(raw_spinlock_irqsave)(&cxl_cper_work_lock);
>  
>  	if (!cxl_cper_work)
>  		return;

[Severity: High]
This is a similar pre-existing issue. In cxl_cper_post_event() (just below
this quoted hunk), the code copies the payload using a fixed size:

	wd.event_type = event_type;
	memcpy(&wd.rec, rec, sizeof(wd.rec));

Could this copy out-of-bounds memory if the firmware provided a payload
where the outer envelope (gdata->error_data_length) is smaller than
sizeof(wd.rec)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803221810.3685703-1-terry.bowman@amd.com?part=3