On Wed, 29 Apr 2026 at 13:46, Jia Jia <[email protected]> wrote:
>
> Commit c1c4d6b38b13 added offset + length checks for the
> patrol_scrub and ecs Set Feature branches, but the remaining
> branches still copy mailbox payload data into fixed-size
> write-attribute objects without the same validation.
>
> A full mailbox payload can still reach rank_sparing and overrun
> CXLMemSparingWriteAttrs on current master. With an ASan build
> this aborts the host process with:
>
>   ERROR: AddressSanitizer: heap-buffer-overflow
>   WRITE of size 2016
>       #0 __interceptor_memcpy
>       #1 cmd_features_set_feature ../hw/cxl/cxl-mailbox-utils.c:1908
>       #2 cxl_process_cci_message ../hw/cxl/cxl-mailbox-utils.c:4622
>       #3 mailbox_reg_write ../hw/cxl/cxl-device-utils.c:209
>
> Apply the same offset + length validation to soft_ppr,
> hard_ppr, cacheline_sparing, row_sparing, bank_sparing, and
> rank_sparing so oversized requests fail with
> CXL_MBOX_INVALID_PAYLOAD_LENGTH instead of overflowing the
> write-attribute buffers.
>
> Add a qtest covering the rank_sparing path.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3458
> Signed-off-by: Jia Jia <[email protected]>

I don't think we should mix refactoring with this bug fix,
but I do notice there's a lot of very repetitive looking
code in this function, which is why the patch has to add a
length check in six different places, and any new feature
type will have to add another one. maybe there's a way to make
it less awkwardly repetitive...

thanks
-- PMM

Reply via email to