Re: [PATCH] mailbox: pcc: validate PCCT subspaces before parsing

[Jassi Brar]

On Fri, Oct 16, 2026 at 9:15 AM Jiale Yao <yaojiale02@xxxxxxx> wrote:
>
> The PCC mailbox probe walks PCCT entries by their declared lengths
> without checking that each entry is contained in the table. It also
> passes generic headers to type-specific parsers without establishing the
> required record size. Malformed firmware can therefore move the cursor
> outside the PCCT or cause out-of-bounds field reads.
>
> Validate the table header, each subspace range, and the minimum size for
> supported subspace types during both the counting and probe passes.
>
> Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
> ---
>  drivers/mailbox/pcc.c | 53 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 9888dab64639..37b2ddb44bda 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -553,6 +553,42 @@ static const struct mbox_chan_ops pcc_chan_ops = {
>         .last_tx_done = pcc_last_tx_done,
>  };
>
> +static bool pcc_subspace_is_valid(struct acpi_subtable_header *entry,
> +                                 unsigned long end)
> +{
> +       unsigned long start = (unsigned long)entry;
> +       size_t length, min_length;
> +
> +       if (start >= end || end - start < sizeof(*entry))
> +               return false;

(end - start < sizeof(*entry)) is implied by the two length  checks
below, so only "start >= end" is needed here.

> +       length = entry->length;
> +       if (length < sizeof(*entry) || length > end - start)
> +               return false;
> +
> +       switch (entry->type) {
> +       case ACPI_PCCT_TYPE_GENERIC_SUBSPACE:
> +               min_length = sizeof(struct acpi_pcct_subspace);
> +               break;
> +       case ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE:
> +               min_length = sizeof(struct acpi_pcct_hw_reduced);
> +               break;
> +       case ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2:
> +               min_length = sizeof(struct acpi_pcct_hw_reduced_type2);
> +               break;
> +       case ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE:
> +               min_length = sizeof(struct acpi_pcct_ext_pcc_master);
> +               break;
> +       case ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE:
> +               min_length = sizeof(struct acpi_pcct_ext_pcc_slave);
> +               break;
> +       default:
> +               return false;
> +       }
> +
> +       return length >= min_length;

How about something like

static const u16 pcc_subspace_min_len[ACPI_PCCT_TYPE_RESERVED] = {
    [ACPI_PCCT_TYPE_GENERIC_SUBSPACE] =
    sizeof(struct acpi_pcct_subspace),
    [ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE] =
    sizeof(struct acpi_pcct_hw_reduced),
    [ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2] =
    sizeof(struct acpi_pcct_hw_reduced_type2),
    [ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE] =
    sizeof(struct acpi_pcct_ext_pcc_master),
    [ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE] =
    sizeof(struct acpi_pcct_ext_pcc_slave),
};

return entry->length >= pcc_subspace_min_len[entry->type];

Cheers
Jassi