[PATCH v2 2/7] ACPI: APEI: GHES: Validate CXL protocol error section length before RAS cap copy
Dave Jiang <[email protected]>
| Newsgroups | org.kernel.vger.linux-cxl,org.kernel.vger.linux-acpi |
|---|---|
| Message-ID | <[email protected]> |
cxl_cper_setup_prot_err_work_data() locates the RAS Capability block by skipping a firmware-controlled DVSEC: dvsec_start = (u8 *)(prot_err + 1); cap_start = dvsec_start + prot_err->dvsec_len; memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap)); prot_err->dvsec_len is taken from the CPER section and is never validated. Add a check to cxl_cper_sec_prot_err_valid() to first verify the section is large enough to contain the header struct itself before accessing any fields, and then to verify the header, DVSEC, and RAS Capability block all fit 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/[email protected]/ Fixes: 315c2f0b53ba ("acpi/ghes, cper: Recognize and cache CXL Protocol errors") Assisted-by: Claude:claude-sonnet-4-6 Reviewed-by: Ben Cheatham <[email protected]> Signed-off-by: Dave Jiang <[email protected]> --- v2: - Missing bounds check on CPER record length and dvsec_len (sashiko) - Add Ben's reviewed-by --- drivers/acpi/acpi_extlog.c | 7 ++++--- drivers/acpi/apei/ghes.c | 7 ++++--- drivers/acpi/apei/ghes_helpers.c | 22 +++++++++++++++++++++- include/cxl/event.h | 4 ++-- 4 files changed, 31 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c index 7ad3b36013cc..06a944dadbc1 100644 --- a/drivers/acpi/acpi_extlog.c +++ b/drivers/acpi/acpi_extlog.c @@ -165,12 +165,12 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err, static void extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err, - int severity) + int severity, u32 len) { #ifdef 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; if (cxl_cper_setup_prot_err_work_data(&wd, prot_err, severity)) @@ -236,7 +236,8 @@ static int extlog_print(struct notifier_block *nb, unsigned long val, acpi_hest_get_payload(gdata); extlog_cxl_cper_handle_prot_err(prot_err, - gdata->error_severity); + gdata->error_severity, + gdata->error_data_length); } else if (guid_equal(sec_type, &CPER_SEC_PCIE)) { struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata); diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 9916271a8151..6fa5d3bab1ce 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; static 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); @@ -956,7 +956,8 @@ static void ghes_do_proc(struct ghes *ghes, } else if (guid_equal(sec_type, &CPER_SEC_CXL_PROT_ERR)) { struct cxl_cper_sec_prot_err *prot_err = acpi_hest_get_payload(gdata); - cxl_cper_post_prot_err(prot_err, gdata->error_severity); + cxl_cper_post_prot_err(prot_err, gdata->error_severity, + gdata->error_data_length); } else if (guid_equal(sec_type, &CPER_SEC_CXL_GEN_MEDIA_GUID)) { struct cxl_cper_event_rec *rec = acpi_hest_get_payload(gdata); diff --git a/drivers/acpi/apei/ghes_helpers.c b/drivers/acpi/apei/ghes_helpers.c index bc7111b740af..7db6a6722fcf 100644 --- a/drivers/acpi/apei/ghes_helpers.c +++ b/drivers/acpi/apei/ghes_helpers.c @@ -5,8 +5,15 @@ #include <linux/aer.h> #include <cxl/event.h> -int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err) +int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err, u32 len) { + if (len < sizeof(*prot_err)) { + pr_err_ratelimited(FW_WARN + "CXL CPER prot err section too small (%u)\n", + len); + return -EINVAL; + } + if (!(prot_err->valid_bits & PROT_ERR_VALID_AGENT_ADDRESS)) { pr_err_ratelimited("CXL CPER invalid agent type\n"); return -EINVAL; @@ -23,6 +30,19 @@ int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err) return -EINVAL; } + /* + * The RAS Capability block follows a firmware-controlled DVSEC of + * prot_err->dvsec_len bytes. Verify the header, the DVSEC and the RAS + * Capability block all fit within the CPER section. + */ + if (sizeof(*prot_err) + prot_err->dvsec_len + + sizeof(struct cxl_ras_capability_regs) > len) { + pr_err_ratelimited(FW_WARN + "CXL CPER prot err section too small (%u)\n", + len); + return -EINVAL; + } + if ((prot_err->agent_type == RCD || prot_err->agent_type == DEVICE || prot_err->agent_type == LD || prot_err->agent_type == FMLD) && !(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER)) diff --git a/include/cxl/event.h b/include/cxl/event.h index ff97fea718d2..912305bee3bc 100644 --- a/include/cxl/event.h +++ b/include/cxl/event.h @@ -321,13 +321,13 @@ static inline int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data #endif #ifdef CONFIG_ACPI_APEI_PCIEAER -int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err); +int cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err, u32 len); int cxl_cper_setup_prot_err_work_data(struct cxl_cper_prot_err_work_data *wd, struct cxl_cper_sec_prot_err *prot_err, int severity); #else static inline int -cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err) +cxl_cper_sec_prot_err_valid(struct cxl_cper_sec_prot_err *prot_err, u32 len) { return -EOPNOTSUPP; } -- 2.55.0