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

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

[Jiale Yao]

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@163.com>
---
 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;
+
+	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;
+}
+
 /**
  * parse_pcc_subspace - Count PCC subspaces defined
  * @header: Pointer to the ACPI subtable header under the PCCT.
@@ -568,6 +604,9 @@ static int parse_pcc_subspace(union acpi_subtable_headers *header,
 {
 	struct acpi_pcct_subspace *ss = (struct acpi_pcct_subspace *) header;
 
+	if (!pcc_subspace_is_valid(&ss->header, end))
+		return -EINVAL;
+
 	if (ss->header.type < ACPI_PCCT_TYPE_RESERVED)
 		return 0;
 
@@ -812,6 +851,7 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 	struct acpi_table_header *pcct_tbl;
 	struct acpi_subtable_header *pcct_entry;
 	struct acpi_table_pcct *acpi_pcct_tbl;
+	unsigned long table_end;
 	acpi_status status = AE_OK;
 	int i, rc, count = pcc_chan_count;
 
@@ -821,6 +861,13 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 	if (ACPI_FAILURE(status) || !pcct_tbl)
 		return -ENODEV;
 
+	acpi_pcct_tbl = (struct acpi_table_pcct *)pcct_tbl;
+	if (pcct_tbl->length < sizeof(*acpi_pcct_tbl)) {
+		rc = -EINVAL;
+		goto err;
+	}
+	table_end = (unsigned long)pcct_tbl + pcct_tbl->length;
+
 	pcc_mbox_channels = devm_kcalloc(dev, count, sizeof(*pcc_mbox_channels),
 					 GFP_KERNEL);
 	if (!pcc_mbox_channels) {
@@ -844,7 +891,6 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 	pcct_entry = (struct acpi_subtable_header *) (
 		(unsigned long) pcct_tbl + sizeof(struct acpi_table_pcct));
 
-	acpi_pcct_tbl = (struct acpi_table_pcct *) pcct_tbl;
 	if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL) {
 		pcc_mbox_ctrl->txdone_irq = true;
 		pcc_mbox_ctrl->txdone_poll = false;
@@ -856,6 +902,11 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 	for (i = 0; i < count; i++) {
 		struct pcc_chan_info *pchan = chan_info + i;
 
+		if (!pcc_subspace_is_valid(pcct_entry, table_end)) {
+			rc = -EINVAL;
+			goto err;
+		}
+
 		pcc_mbox_channels[i].con_priv = pchan;
 		pchan->chan.mchan = &pcc_mbox_channels[i];
 
-- 
2.34.1

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

[jiale yao]

At 2026-10-16 22:15:30, "Jiale Yao" <yaojiale02@163.com> 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.
>

Fixes: 86c22f8c9a3b ("Mailbox: Add support for Platform Communication Channel")

>Signed-off-by: Jiale Yao <yaojiale02@163.com>
>---
> 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;
>+
>+	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;
>+}
>+
> /**
>  * parse_pcc_subspace - Count PCC subspaces defined
>  * @header: Pointer to the ACPI subtable header under the PCCT.
>@@ -568,6 +604,9 @@ static int parse_pcc_subspace(union acpi_subtable_headers *header,
> {
> 	struct acpi_pcct_subspace *ss = (struct acpi_pcct_subspace *) header;
> 
>+	if (!pcc_subspace_is_valid(&ss->header, end))
>+		return -EINVAL;
>+
> 	if (ss->header.type < ACPI_PCCT_TYPE_RESERVED)
> 		return 0;
> 
>@@ -812,6 +851,7 @@ static int pcc_mbox_probe(struct platform_device *pdev)
> 	struct acpi_table_header *pcct_tbl;
> 	struct acpi_subtable_header *pcct_entry;
> 	struct acpi_table_pcct *acpi_pcct_tbl;
>+	unsigned long table_end;
> 	acpi_status status = AE_OK;
> 	int i, rc, count = pcc_chan_count;
> 
>@@ -821,6 +861,13 @@ static int pcc_mbox_probe(struct platform_device *pdev)
> 	if (ACPI_FAILURE(status) || !pcct_tbl)
> 		return -ENODEV;
> 
>+	acpi_pcct_tbl = (struct acpi_table_pcct *)pcct_tbl;
>+	if (pcct_tbl->length < sizeof(*acpi_pcct_tbl)) {
>+		rc = -EINVAL;
>+		goto err;
>+	}
>+	table_end = (unsigned long)pcct_tbl + pcct_tbl->length;
>+
> 	pcc_mbox_channels = devm_kcalloc(dev, count, sizeof(*pcc_mbox_channels),
> 					 GFP_KERNEL);
> 	if (!pcc_mbox_channels) {
>@@ -844,7 +891,6 @@ static int pcc_mbox_probe(struct platform_device *pdev)
> 	pcct_entry = (struct acpi_subtable_header *) (
> 		(unsigned long) pcct_tbl + sizeof(struct acpi_table_pcct));
> 
>-	acpi_pcct_tbl = (struct acpi_table_pcct *) pcct_tbl;
> 	if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL) {
> 		pcc_mbox_ctrl->txdone_irq = true;
> 		pcc_mbox_ctrl->txdone_poll = false;
>@@ -856,6 +902,11 @@ static int pcc_mbox_probe(struct platform_device *pdev)
> 	for (i = 0; i < count; i++) {
> 		struct pcc_chan_info *pchan = chan_info + i;
> 
>+		if (!pcc_subspace_is_valid(pcct_entry, table_end)) {
>+			rc = -EINVAL;
>+			goto err;
>+		}
>+
> 		pcc_mbox_channels[i].con_priv = pchan;
> 		pchan->chan.mchan = &pcc_mbox_channels[i];
> 
>-- 
>2.34.1

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

[Jassi Brar]

On Fri, Oct 16, 2026 at 9:15 AM Jiale Yao <yaojiale02@163.com> 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@163.com>
> ---
>  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

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

[jiale yao]

Hi Jassi,
At 2026-10-22 10:26:27, "Jassi Brar" <jassisinghbrar@gmail.com> wrote:
>On Fri, Oct 16, 2026 at 9:15 AM Jiale Yao <yaojiale02@163.com> 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@163.com>
>> ---
>>  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),
>};

Sounds great, I will submit v2.

Thanks for your review.

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

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

[jiale yao]

At 2026-10-22 10:42:19, "jiale yao" <19888972804@163.com> wrote:
>Hi Jassi,
>At 2026-10-22 10:26:27, "Jassi Brar" <jassisinghbrar@gmail.com> wrote:
>>On Fri, Oct 16, 2026 at 9:15 AM Jiale Yao <yaojiale02@163.com> 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@163.com>
>>> ---
>>>  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),
>>};
>
>Sounds great, I will submit v2.

v2 is in, https://lore.kernel.org/all/20261023093839.715037-1-yaojiale02@163.com/

>
>Thanks for your review.
>
>>
>>return entry->length >= pcc_subspace_min_len[entry->type];
>>
>>Cheers
>>Jassi