mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mailbox: pcc: validate PCCT subspaces before parsing
@ 2026-10-16 14:15 Jiale Yao
  2026-10-17  7:35 ` jiale yao
  0 siblings, 1 reply; 2+ messages in thread
From: Jiale Yao @ 2026-10-16 14:15 UTC (permalink / raw)
  To: Sudeep Holla, Jassi Brar, linux-acpi, linux-kernel; +Cc: 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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re:[PATCH] mailbox: pcc: validate PCCT subspaces before parsing
  2026-10-16 14:15 [PATCH] mailbox: pcc: validate PCCT subspaces before parsing Jiale Yao
@ 2026-10-17  7:35 ` jiale yao
  0 siblings, 0 replies; 2+ messages in thread
From: jiale yao @ 2026-10-17  7:35 UTC (permalink / raw)
  To: Sudeep Holla, Jassi Brar, linux-acpi, linux-kernel

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-17  7:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-16 14:15 [PATCH] mailbox: pcc: validate PCCT subspaces before parsing Jiale Yao
2026-10-17  7:35 ` jiale yao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®