Re: [PATCH v2 01/17] ACPI: GTDT: Account for GTDTv3 size when walking the platform timer descriptors

[Marc Zyngier <maz@kernel.org>]

On Fri, 15 May 2026 10:51:52 +0100,
Sudeep Holla <sudeep.holla@kernel.org> wrote:
> 
> On Thu, May 14, 2026 at 04:09:29PM +0100, Marc Zyngier wrote:
> > Since ARMv8.1, the architecture has grown an EL2-private virtual
> > timer. This has been described in ACPI since ACPI v6.3 and revision
> > 3 of the GTDT table.
> > 
> > An aditional structure was added in ACPICA, though in a rather
> > bizarre way, and merged in v5.1 as 8f5a14d053100 ("ACPICA: ACPI 6.3:
> > add GTDT Revision 3 support").
> > 
> > Finally plug the table parsing in GTDT, and correct the parsing of
> > the platform timer subtables to account for the expanded size of
> > the base table.
> > 
> > Suggested-by: Sudeep Holla <sudeep.holla@kernel.org>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  drivers/acpi/arm64/gtdt.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c
> > index ffc867bac2d60..b9d9b8edf2df7 100644
> > --- a/drivers/acpi/arm64/gtdt.c
> > +++ b/drivers/acpi/arm64/gtdt.c
> > @@ -32,6 +32,12 @@ struct acpi_gtdt_descriptor {
> >  	struct acpi_table_gtdt *gtdt;
> >  	void *gtdt_end;
> >  	void *platform_timer;
> > +	bool v3;
> > +};
> > +
> > +struct gtdt_v3 {
> > +	struct acpi_table_gtdt	gtdt_v2;
> > +	struct acpi_gtdt_el2	el2_vtimer;
> >  };
> >  
> >  static struct acpi_gtdt_descriptor acpi_gtdt_desc __initdata;
> > @@ -39,8 +45,14 @@ static struct acpi_gtdt_descriptor acpi_gtdt_desc __initdata;
> >  static __init bool platform_timer_valid(void *platform_timer)
> >  {
> >  	struct acpi_gtdt_header *gh = platform_timer;
> > +	void *platform_timer_begin;
> > +
> > +	if (acpi_gtdt_desc.v3)
> > +		platform_timer_begin = container_of(acpi_gtdt_desc.gtdt, struct gtdt_v3, gtdt_v2) + 1;
> > +	else
> > +		platform_timer_begin = acpi_gtdt_desc.gtdt + 1;
> >
> > -	return (platform_timer >= (void *)(acpi_gtdt_desc.gtdt + 1) &&
> > +	return (platform_timer >= platform_timer_begin &&
> >  		platform_timer < acpi_gtdt_desc.gtdt_end &&
> >  		gh->length != 0 &&
> >  		platform_timer + gh->length <= acpi_gtdt_desc.gtdt_end);
> > @@ -169,6 +181,7 @@ int __init acpi_gtdt_init(struct acpi_table_header *table,
> >  	acpi_gtdt_desc.gtdt = gtdt;
> >  	acpi_gtdt_desc.gtdt_end = (void *)table + table->length;
> >  	acpi_gtdt_desc.platform_timer = NULL;
> > +	acpi_gtdt_desc.v3 = gtdt->header.revision >= 3 && gtdt->header.length >= sizeof(struct gtdt_v3);
> 
> Regarding Sashiko’s comment about the missing length validation for GTDT v2, I
> realised that the current check could cause a malformed v3 table to be
> interpreted as v2 if its length does not match the expected v3
> length.

Yeah, that's overall dodgy. As much as I hate having to write a
validating parser for ACPI, we need to be prepared for the worst.

> It would be better to fail early and return an error rather than allow
> processing to continue with the table incorrectly interpreted as v2.

How about something like the hack below?

Thanks,

	M.

diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c
index 12bc8875e95e2..ceec69609f038 100644
--- a/drivers/acpi/arm64/gtdt.c
+++ b/drivers/acpi/arm64/gtdt.c
@@ -202,7 +202,15 @@ int __init acpi_gtdt_init(struct acpi_table_header *table,
 	acpi_gtdt_desc.gtdt = gtdt;
 	acpi_gtdt_desc.gtdt_end = (void *)table + table->length;
 	acpi_gtdt_desc.platform_timer = NULL;
-	acpi_gtdt_desc.v3 = gtdt->header.revision >= 3 && gtdt->header.length >= sizeof(struct gtdt_v3);
+
+	if ((gtdt->header.revision >= 3 && gtdt->header.length < sizeof(struct gtdt_v3)) ||
+	    (gtdt->header.revision == 2 && gtdt->header.length < sizeof(*gtdt))) {
+		pr_err(FW_BUG "GTDT with invalid size %d\n", gtdt->header.length);
+		return -EINVAL;
+	}
+
+	acpi_gtdt_desc.v3 = gtdt->header.revision >= 3;
+
 	if (platform_timer_count)
 		*platform_timer_count = 0;
 

-- 
Without deviation from the norm, progress is not possible.