Re: [PATCH v3 00/10] ACPI: APEI: GHES: Collection of fixes for issues reported by sashiko

On Tue, Jul 21, 2026 at 5:06 PM Dave Jiang <[email protected]> wrote:
>
>
>
> On 7/21/26 4:33 AM, Rafael J. Wysocki (Intel) wrote:
> > On Fri, Jul 17, 2026 at 6:16 PM Dave Jiang <[email protected]> wrote:
> >>
> >> A collection of fixes for pre-existing issues reported by sashiko-bot while
> >> reviewing patches.
> >>
> >> The v1 posting fixed a first batch of these issues, and v2 grew the set as
> >> sashiko-bot's review surfaced more problems in the same CPER/extlog paths.
> >> sashiko-bot's review of v2 flagged yet more of the same class of bug in
> >> neighbouring code, so v3 adds those as well: an out-of-range section length
> >> that defeats cper_estatus_check(), the same unvalidated AER buffer handling
> >> in ghes_handle_aer() that was already fixed for extlog, and an unbounded
> >> walk of the extlog record.
> >>
> >> 1/10:  Bound the CXL event record copy to the firmware section length.
> >> 2/10:  Reject CPER records with an out-of-range error_data_length.
> >> 3/10:  Validate the CXL protocol error section length before the RAS cap copy.
> >> 4/10:  Avoid populating software AER metadata from the raw hardware buffer.
> >> 5/10:  Validate the PCIe error section length before payload access.
> >> 6/10:  Fix the CONFIG_ACPI_APEI_PCIEAER guard typo in extlog.c.
> >> 7/10:  Defer CXL protocol error handling to avoid a lock inversion.
> >> 8/10:  Validate the memory error section length before payload access.
> >> 9/10:  Bound the AER info copy and sanitize software metadata in ghes.c.
> >> 10/10: Validate the extlog record length before walking sections.
> >>
> >> Note patch 2 touches drivers/firmware/efi/cper.c; it closes the length
> >> validation hole at the shared choke point that the per-section guards in the
> >> rest of the series rely on.
> >>
> >> sashiko-bot's review of v2 also flagged a few related issues that are not
> >> addressed here because they live in other subsystems.
> >>
> >>   - cxl_cper_print_prot_err() in drivers/firmware/efi/cper_cxl.c uses the
> >>     firmware-controlled dvsec_len without bounding it against the section
> >>     length.
> >>   - cxl_rch_get_aer_info() in drivers/cxl/core/ras_rch.c reads the RCH AER
> >>     capability from MMIO into a struct aer_capability_regs without clearing
> >>     the software-only header_len/flit fields, the same class of issue as
> >>     patches 4 and 9.
> >>
> >> v1: https://lore.kernel.org/linux-cxl/[email protected]/
> >> v2: https://lore.kernel.org/linux-cxl/[email protected]/
> >>
> >> Changes since v2
> >> ----------------
> >> - Dropped the standalone spin_lock_irqsave() change; it is a separate issue
> >>   being handled by Terry Bowman.
> >> - New patch 2: reject a section whose error_data_length is out of range in
> >>   cper_estatus_check(), closing the signed-int overflow that let a crafted
> >>   section bypass the check and defeat the per-caller size guards (sashiko).
> >> - New patch 9: apply the same AER buffer sanitization to ghes_handle_aer()
> >>   that patch 4 applies to extlog (sashiko).
> >> - New patch 10: bound the extlog record and run cper_estatus_check() before
> >>   walking its sections, which extlog did not do (sashiko).
> >> - Patch 8: move the memory error length check into ghes_do_proc() so the
> >>   report chain and arch reporter are covered too, and bound against the
> >>   older UEFI 2.1/2.2 layout rather than the full struct (sashiko).
> >> - Reordered so the extlog PCIe path is hardened, and the CXL protocol error
> >>   handling is deferred to the workqueue, before the typo fix that activates
> >>   that code.
> >>
> >> Changes since v1
> >> ----------------
> >> - See the v2 posting for the full v1 -> v2 changelog.
> >>
> >>
> >> Dave Jiang (10):
> >>   ACPI: APEI: GHES: Bound CXL event record copy to the firmware section
> >>     length
> >>   efi/cper: Reject CPER records with an out-of-range error_data_length
> >>   ACPI: APEI: GHES: Validate CXL protocol error section length before
> >>     RAS cap copy
> >>   ACPI: extlog: Avoid populating software AER metadata from raw hardware
> >>     buffer
> >>   ACPI: extlog: Validate PCIe error section length before payload access
> >>   ACPI: extlog: Defer CXL protocol error handling to avoid lock
> >>     inversion
> >>   ACPI: extlog: Fix CONFIG_ACPI_APEI_PCIEAER guard typo
> >>   ACPI: APEI: GHES: Validate memory error section length before payload
> >>     access
> >>   ACPI: APEI: GHES: Bound AER info copy and sanitize software metadata
> >>   ACPI: extlog: Validate elog record length before walking sections
> >>
> >>  drivers/acpi/acpi_extlog.c       | 47 ++++++++++++-------------
> >>  drivers/acpi/apei/ghes.c         | 59 +++++++++++++++++++++++++-------
> >>  drivers/acpi/apei/ghes_helpers.c | 21 +++++++++++-
> >>  drivers/firmware/efi/cper.c      | 10 ++++++
> >>  include/acpi/ghes.h              |  4 +++
> >>  include/cxl/event.h              |  4 +--
> >>  6 files changed, 106 insertions(+), 39 deletions(-)
> >
> > All of this looks reasonable to me, but it really is material for Tony
> > to look at.
>
> Thanks Rafael. I've asked Tony to take a look. Looks like he's on vacation so hopefully soon.

After Shuai Xue's review this is good to go from my perspective, but
please address the review feedback.