[PATCH v5 01/13] efi/cper: Reject CPER records with an out-of-range error_data_length
Dave Jiang <[email protected]>
| Newsgroups | org.kernel.vger.linux-acpi,org.kernel.vger.linux-cxl |
|---|---|
| Message-ID | <[email protected]> |
cper_estatus_check() sizes each section with acpi_hest_get_record_size(), which adds the firmware-controlled u32 error_data_length to the header size as a signed int (see <acpi/ghes.h>). A value in the top sizeof(*gdata) bytes of the u32 range wraps the sum small rather than large, so it slips past the "record_size > data_len" check: against the 72-byte v300 header, 0xffffffb9 sizes the record at 1 and acpi_hest_get_next() walks it a byte at a time, off the end. 0xffffffb8 sizes it at 0 and loops forever. Use check_add_overflow() to reject a sum that will not fit the int those helpers return, since the walk advances by that value. That subsumes the "acpi_hest_get_size(gdata) > data_len" test above it, because record_size is never smaller than the header. The later "len < sizeof(*foo)" guards rely on this per-section upper bound; they are lower bounds only. Reported-by: [email protected] Closes: https://sashiko.dev/#/patchset/[email protected]?part=1 Fixes: 45b14a4ffcc1 ("efi: cper: Fix possible out-of-bounds access") Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Dave Jiang <[email protected]> --- v5: - Use check_add_overflow() rather than a u64 sum plus an INT_MAX test, and drop the now-redundant acpi_hest_get_size() bound (Jonathan Cameron). --- drivers/firmware/efi/cper.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c index 06b4fdb59917..332c78f2d561 100644 --- a/drivers/firmware/efi/cper.c +++ b/drivers/firmware/efi/cper.c @@ -752,7 +752,7 @@ EXPORT_SYMBOL_GPL(cper_estatus_check_header); int cper_estatus_check(const struct acpi_hest_generic_status *estatus) { struct acpi_hest_generic_data *gdata; - unsigned int data_len, record_size; + unsigned int data_len; int rc; rc = cper_estatus_check_header(estatus); @@ -762,10 +762,18 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus) data_len = estatus->data_length; apei_estatus_for_each_section(estatus, gdata) { - if (acpi_hest_get_size(gdata) > data_len) + int record_size; + + /* + * The <acpi/ghes.h> helpers sum these as a signed int, so a + * huge error_data_length wraps small rather than large and the + * walk then advances by that wrapped value. Reject a size an + * int cannot carry. + */ + if (check_add_overflow(acpi_hest_get_size(gdata), + gdata->error_data_length, &record_size)) return -EINVAL; - record_size = acpi_hest_get_record_size(gdata); if (record_size > data_len) return -EINVAL; -- 2.54.0