Re: [PATCH] cxl/pci: Fix out-of-bounds read of the RAS Header Log

[sashiko-bot@kernel.org]

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

New issues:
- [Low] Tracepoint comments still reference 512B headerlog data despite the size reduction.

Pre-existing issues:
- [High] Missing bounds check on CPER record length and `dvsec_len` in CXL protocol error handling, leading to out-of-bounds read and information leak.
--

commit c3e9e84c0d6db804f921aa7a18334a01bcf77999
Author: Richard Cheng <icheng@nvidia.com>
cxl/pci: Fix out-of-bounds read of the RAS Header Log

This commit reduces the CXL_HEADERLOG_SIZE constant from 512 bytes to 64
bytes to match the actual size of the CXL RAS Header Log. It prevents an
out-of-bounds read and subsequent kernel memory leak that occurs when
the CPER path tracepoints copy 512 bytes from a smaller heap allocation.

> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 1297594beaec3..f322d7c79ed22 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -158,8 +158,8 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
>  #define CXL_RAS_CAP_CONTROL_FE_MASK GENMASK(5, 0)
>  #define CXL_RAS_HEADER_LOG_OFFSET 0x18
>  #define CXL_RAS_CAPABILITY_LENGTH 0x58
> -#define CXL_HEADERLOG_SIZE SZ_512
> -#define CXL_HEADERLOG_SIZE_U32 SZ_512 / sizeof(u32)
> +#define CXL_HEADERLOG_SIZE SZ_64
> +#define CXL_HEADERLOG_SIZE_U32 (CXL_HEADERLOG_SIZE / sizeof(u32))

[Severity: Low]
Are the tracepoint comments now stale?

Looking at cxl_port_aer_uncorrectable_error() and
cxl_aer_uncorrectable_error() in drivers/cxl/core/trace.h, the comments
still state:

    /*
     * Embed the 512B headerlog data for user app retrieval and
     * parsing, but no need to print this in the trace buffer.
     */
    memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);

[Severity: High]
This is a pre-existing issue, but while looking at the CPER path this
commit fixes, is there a missing bounds check on the CPER record length
and dvsec_len?

Looking at cxl_cper_setup_prot_err_work_data() in
drivers/acpi/apei/ghes_helpers.c, cap_start is calculated using an
unvalidated dvsec_len:

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

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

Similarly, in cxl_cper_print_prot_err() in drivers/firmware/efi/cper_cxl.c:

    print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, (prot_err + 1),
                   prot_err->dvsec_len, 0);

And in ghes_do_proc() in drivers/acpi/apei/ghes.c:

    struct cxl_cper_sec_prot_err *prot_err = acpi_hest_get_payload(gdata);
    cxl_cper_post_prot_err(prot_err, gdata->error_severity);

If gdata->error_data_length is not checked to ensure it contains the CXL
protocol error structure, or if a malformed firmware record specifies a
large dvsec_len, could this lead to out-of-bounds reads and leak
sensitive kernel memory into dmesg?

>  
>  /* CXL 2.0 8.2.8.1 Device Capabilities Array Register */
>  #define CXLDEV_CAP_ARRAY_OFFSET 0x0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605041602.37944-1-icheng@nvidia.com?part=1