[PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[Kees Cook]

Functionally identical to ACPICA upstream pull request 813:
https://github.com/acpica/acpica/pull/813

One-element arrays (and multi-element arrays being treated as
dynamically sized) are deprecated[1] and are being replaced with
flexible array members in support of the ongoing efforts to tighten the
FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.

Replace one-element array with flexible-array member in struct
acpi_resource_extended_irq. Replace 4-byte fixed-size array with 4-byte
padding in a union with a flexible-array member in struct
acpi_pci_routing_table.

This results in no differences in binary output.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Robert Moore <robert.moore@intel.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: linux-acpi@vger.kernel.org
Cc: devel@acpica.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/acpi/acrestyp.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h
index a7fb8ddb3dc6..ee945084d46e 100644
--- a/include/acpi/acrestyp.h
+++ b/include/acpi/acrestyp.h
@@ -332,7 +332,7 @@ struct acpi_resource_extended_irq {
 	u8 wake_capable;
 	u8 interrupt_count;
 	struct acpi_resource_source resource_source;
-	u32 interrupts[1];
+	u32 interrupts[];
 };
 
 struct acpi_resource_generic_register {
@@ -679,7 +679,10 @@ struct acpi_pci_routing_table {
 	u32 pin;
 	u64 address;		/* here for 64-bit alignment */
 	u32 source_index;
-	char source[4];		/* pad to 64 bits so sizeof() works in all cases */
+	union {
+		char pad[4];	/* pad to 64 bits so sizeof() works in all cases */
+		DECLARE_FLEX_ARRAY(char, source);
+	};
 };
 
 #endif				/* __ACRESTYP_H__ */
-- 
2.34.1

Re: [PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[Kees Cook]

On Fri, Nov 18, 2022 at 10:15:51AM -0800, Kees Cook wrote:
> Functionally identical to ACPICA upstream pull request 813:
> https://github.com/acpica/acpica/pull/813

Any update on this? Upstream is currently unbuildable since October.

> One-element arrays (and multi-element arrays being treated as
> dynamically sized) are deprecated[1] and are being replaced with
> flexible array members in support of the ongoing efforts to tighten the
> FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> 
> Replace one-element array with flexible-array member in struct
> acpi_resource_extended_irq. Replace 4-byte fixed-size array with 4-byte
> padding in a union with a flexible-array member in struct
> acpi_pci_routing_table.
> 
> This results in no differences in binary output.

In the meantime, can you take this patch for Linux, and we can wait for
ACPICA to catch up?

Thanks!

-Kees

> 
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Cc: devel@acpica.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/acpi/acrestyp.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h
> index a7fb8ddb3dc6..ee945084d46e 100644
> --- a/include/acpi/acrestyp.h
> +++ b/include/acpi/acrestyp.h
> @@ -332,7 +332,7 @@ struct acpi_resource_extended_irq {
>  	u8 wake_capable;
>  	u8 interrupt_count;
>  	struct acpi_resource_source resource_source;
> -	u32 interrupts[1];
> +	u32 interrupts[];
>  };
>  
>  struct acpi_resource_generic_register {
> @@ -679,7 +679,10 @@ struct acpi_pci_routing_table {
>  	u32 pin;
>  	u64 address;		/* here for 64-bit alignment */
>  	u32 source_index;
> -	char source[4];		/* pad to 64 bits so sizeof() works in all cases */
> +	union {
> +		char pad[4];	/* pad to 64 bits so sizeof() works in all cases */
> +		DECLARE_FLEX_ARRAY(char, source);
> +	};
>  };
>  
>  #endif				/* __ACRESTYP_H__ */
> -- 
> 2.34.1
> 

-- 
Kees Cook

Re: [PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[Rafael J. Wysocki]

On Sun, Jan 15, 2023 at 7:38 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Nov 18, 2022 at 10:15:51AM -0800, Kees Cook wrote:
> > Functionally identical to ACPICA upstream pull request 813:
> > https://github.com/acpica/acpica/pull/813
>
> Any update on this? Upstream is currently unbuildable since October.
>
> > One-element arrays (and multi-element arrays being treated as
> > dynamically sized) are deprecated[1] and are being replaced with
> > flexible array members in support of the ongoing efforts to tighten the
> > FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> > with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> >
> > Replace one-element array with flexible-array member in struct
> > acpi_resource_extended_irq. Replace 4-byte fixed-size array with 4-byte
> > padding in a union with a flexible-array member in struct
> > acpi_pci_routing_table.
> >
> > This results in no differences in binary output.
>
> In the meantime, can you take this patch for Linux, and we can wait for
> ACPICA to catch up?

Applied now (as 6.3 material), sorry for the delay.

Thanks!

> >
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: Robert Moore <robert.moore@intel.com>
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: linux-acpi@vger.kernel.org
> > Cc: devel@acpica.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  include/acpi/acrestyp.h | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h
> > index a7fb8ddb3dc6..ee945084d46e 100644
> > --- a/include/acpi/acrestyp.h
> > +++ b/include/acpi/acrestyp.h
> > @@ -332,7 +332,7 @@ struct acpi_resource_extended_irq {
> >       u8 wake_capable;
> >       u8 interrupt_count;
> >       struct acpi_resource_source resource_source;
> > -     u32 interrupts[1];
> > +     u32 interrupts[];
> >  };
> >
> >  struct acpi_resource_generic_register {
> > @@ -679,7 +679,10 @@ struct acpi_pci_routing_table {
> >       u32 pin;
> >       u64 address;            /* here for 64-bit alignment */
> >       u32 source_index;
> > -     char source[4];         /* pad to 64 bits so sizeof() works in all cases */
> > +     union {
> > +             char pad[4];    /* pad to 64 bits so sizeof() works in all cases */
> > +             DECLARE_FLEX_ARRAY(char, source);
> > +     };
> >  };
> >
> >  #endif                               /* __ACRESTYP_H__ */
> > --
> > 2.34.1
> >
>
> --
> Kees Cook

Re: [PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[Kees Cook]

On Fri, Jan 20, 2023 at 07:10:52PM +0100, Rafael J. Wysocki wrote:
> On Sun, Jan 15, 2023 at 7:38 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Fri, Nov 18, 2022 at 10:15:51AM -0800, Kees Cook wrote:
> > > Functionally identical to ACPICA upstream pull request 813:
> > > https://github.com/acpica/acpica/pull/813
> >
> > Any update on this? Upstream is currently unbuildable since October.
> >
> > > One-element arrays (and multi-element arrays being treated as
> > > dynamically sized) are deprecated[1] and are being replaced with
> > > flexible array members in support of the ongoing efforts to tighten the
> > > FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> > > with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> > >
> > > Replace one-element array with flexible-array member in struct
> > > acpi_resource_extended_irq. Replace 4-byte fixed-size array with 4-byte
> > > padding in a union with a flexible-array member in struct
> > > acpi_pci_routing_table.
> > >
> > > This results in no differences in binary output.
> >
> > In the meantime, can you take this patch for Linux, and we can wait for
> > ACPICA to catch up?
> 
> Applied now (as 6.3 material), sorry for the delay.

Thanks!

-- 
Kees Cook

Re: [PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[Rafael J. Wysocki]

On Fri, Jan 20, 2023 at 8:16 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jan 20, 2023 at 07:10:52PM +0100, Rafael J. Wysocki wrote:
> > On Sun, Jan 15, 2023 at 7:38 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Fri, Nov 18, 2022 at 10:15:51AM -0800, Kees Cook wrote:
> > > > Functionally identical to ACPICA upstream pull request 813:
> > > > https://github.com/acpica/acpica/pull/813
> > >
> > > Any update on this? Upstream is currently unbuildable since October.
> > >
> > > > One-element arrays (and multi-element arrays being treated as
> > > > dynamically sized) are deprecated[1] and are being replaced with
> > > > flexible array members in support of the ongoing efforts to tighten the
> > > > FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> > > > with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> > > >
> > > > Replace one-element array with flexible-array member in struct
> > > > acpi_resource_extended_irq. Replace 4-byte fixed-size array with 4-byte
> > > > padding in a union with a flexible-array member in struct
> > > > acpi_pci_routing_table.
> > > >
> > > > This results in no differences in binary output.
> > >
> > > In the meantime, can you take this patch for Linux, and we can wait for
> > > ACPICA to catch up?
> >
> > Applied now (as 6.3 material), sorry for the delay.
>
> Thanks!

Unfortunately, this breaks compilation for the ACPI tools in tools/power/acpi/.

Apparently, the problem is that DECLARE_FLEX_ARRAY() is not defined
when the tools are built, because kernel headers are not used then.

I guess the changes from your upstream PR need to be backported
literally for this to work, so I'll drop this one for the time being.
Or please let me know if you have a better idea.

Re: [PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[Kees Cook]

On Fri, Jan 27, 2023 at 07:08:39PM +0100, Rafael J. Wysocki wrote:
> On Fri, Jan 20, 2023 at 8:16 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Fri, Jan 20, 2023 at 07:10:52PM +0100, Rafael J. Wysocki wrote:
> > > On Sun, Jan 15, 2023 at 7:38 PM Kees Cook <keescook@chromium.org> wrote:
> > > >
> > > > On Fri, Nov 18, 2022 at 10:15:51AM -0800, Kees Cook wrote:
> > > > > Functionally identical to ACPICA upstream pull request 813:
> > > > > https://github.com/acpica/acpica/pull/813
> > > >
> > > > Any update on this? Upstream is currently unbuildable since October.
> > > >
> > > > > One-element arrays (and multi-element arrays being treated as
> > > > > dynamically sized) are deprecated[1] and are being replaced with
> > > > > flexible array members in support of the ongoing efforts to tighten the
> > > > > FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> > > > > with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> > > > >
> > > > > Replace one-element array with flexible-array member in struct
> > > > > acpi_resource_extended_irq. Replace 4-byte fixed-size array with 4-byte
> > > > > padding in a union with a flexible-array member in struct
> > > > > acpi_pci_routing_table.
> > > > >
> > > > > This results in no differences in binary output.
> > > >
> > > > In the meantime, can you take this patch for Linux, and we can wait for
> > > > ACPICA to catch up?
> > >
> > > Applied now (as 6.3 material), sorry for the delay.
> >
> > Thanks!
> 
> Unfortunately, this breaks compilation for the ACPI tools in tools/power/acpi/.

What's the make target to test this?

> Apparently, the problem is that DECLARE_FLEX_ARRAY() is not defined
> when the tools are built, because kernel headers are not used then.

This should exist in the stddef.h tools headers, but perhaps it isn't
included already?

> I guess the changes from your upstream PR need to be backported
> literally for this to work, so I'll drop this one for the time being.
> Or please let me know if you have a better idea.

I can send a new version if I can reproduce the build failure you see...

-- 
Kees Cook

Re: [PATCH] ACPICA: Replace fake flexible arrays with flexible array members

[Rafael J. Wysocki]

On Fri, Jan 27, 2023 at 7:33 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Jan 27, 2023 at 07:08:39PM +0100, Rafael J. Wysocki wrote:
> > On Fri, Jan 20, 2023 at 8:16 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > On Fri, Jan 20, 2023 at 07:10:52PM +0100, Rafael J. Wysocki wrote:
> > > > On Sun, Jan 15, 2023 at 7:38 PM Kees Cook <keescook@chromium.org> wrote:
> > > > >
> > > > > On Fri, Nov 18, 2022 at 10:15:51AM -0800, Kees Cook wrote:
> > > > > > Functionally identical to ACPICA upstream pull request 813:
> > > > > > https://github.com/acpica/acpica/pull/813
> > > > >
> > > > > Any update on this? Upstream is currently unbuildable since October.
> > > > >
> > > > > > One-element arrays (and multi-element arrays being treated as
> > > > > > dynamically sized) are deprecated[1] and are being replaced with
> > > > > > flexible array members in support of the ongoing efforts to tighten the
> > > > > > FORTIFY_SOURCE routines on memcpy(), correctly instrument array indexing
> > > > > > with UBSAN_BOUNDS, and to globally enable -fstrict-flex-arrays=3.
> > > > > >
> > > > > > Replace one-element array with flexible-array member in struct
> > > > > > acpi_resource_extended_irq. Replace 4-byte fixed-size array with 4-byte
> > > > > > padding in a union with a flexible-array member in struct
> > > > > > acpi_pci_routing_table.
> > > > > >
> > > > > > This results in no differences in binary output.
> > > > >
> > > > > In the meantime, can you take this patch for Linux, and we can wait for
> > > > > ACPICA to catch up?
> > > >
> > > > Applied now (as 6.3 material), sorry for the delay.
> > >
> > > Thanks!
> >
> > Unfortunately, this breaks compilation for the ACPI tools in tools/power/acpi/.
>
> What's the make target to test this?

Just cd to tools/power/acpi in the kernel tree and run make.

> > Apparently, the problem is that DECLARE_FLEX_ARRAY() is not defined
> > when the tools are built, because kernel headers are not used then.
>
> This should exist in the stddef.h tools headers, but perhaps it isn't
> included already?

No, it isn't AFAICS.

> > I guess the changes from your upstream PR need to be backported
> > literally for this to work, so I'll drop this one for the time being.
> > Or please let me know if you have a better idea.
>
> I can send a new version if I can reproduce the build failure you see...

Sure, thanks!