From 446d05d5ea77946b8b3b8d0c638d1a446b18503e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 23 Feb 2023 17:12:21 -0800 Subject: ACPI_RESOURCE_VENDOR: Replace 1-element array with flexible array Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). No binary changes appear in the .text nor .data sections. --- source/include/acrestyp.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/include/acrestyp.h b/source/include/acrestyp.h index 10e8ff89f..16356098f 100644 --- a/source/include/acrestyp.h +++ b/source/include/acrestyp.h @@ -357,7 +357,7 @@ typedef struct acpi_resource_fixed_dma typedef struct acpi_resource_vendor { UINT16 ByteLength; - UINT8 ByteData[1]; + UINT8 ByteData[]; } ACPI_RESOURCE_VENDOR; @@ -368,7 +368,7 @@ typedef struct acpi_resource_vendor_typed UINT16 ByteLength; UINT8 UuidSubtype; UINT8 Uuid[ACPI_UUID_LENGTH]; - UINT8 ByteData[1]; + UINT8 ByteData[]; } ACPI_RESOURCE_VENDOR_TYPED; -- cgit v1.2.1 From 151a44c5da640537271a3400c9049511a6e1360f Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 23 Feb 2023 17:12:21 -0800 Subject: ACPI_EFI_FILE_INFO: Replace 1-element array with flexible array Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). No binary changes appear in the .text nor .data sections. --- source/include/platform/acefiex.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/include/platform/acefiex.h b/source/include/platform/acefiex.h index dce3518be..8df422b44 100644 --- a/source/include/platform/acefiex.h +++ b/source/include/platform/acefiex.h @@ -508,7 +508,7 @@ typedef struct { ACPI_EFI_TIME LastAccessTime; ACPI_EFI_TIME ModificationTime; UINT64 Attribute; - CHAR16 FileName[1]; + CHAR16 FileName[]; } ACPI_EFI_FILE_INFO; #define SIZE_OF_ACPI_EFI_FILE_INFO ACPI_OFFSET(ACPI_EFI_FILE_INFO, FileName) -- cgit v1.2.1 From 8c9bd5d151f77767b2fd937911848b7159dc8ee9 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 23 Feb 2023 17:12:21 -0800 Subject: actbl1: Replace 1-element arrays with flexible arrays Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). No .text nor .data differences result from this change. --- source/include/actbl1.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/include/actbl1.h b/source/include/actbl1.h index 2e400b526..63f137117 100644 --- a/source/include/actbl1.h +++ b/source/include/actbl1.h @@ -1272,7 +1272,7 @@ typedef struct acpi_table_drtm typedef struct acpi_drtm_vtable_list { UINT32 ValidatedTableCount; - UINT64 ValidatedTables[1]; + UINT64 ValidatedTables[]; } ACPI_DRTM_VTABLE_LIST; @@ -1291,7 +1291,7 @@ typedef struct acpi_drtm_resource typedef struct acpi_drtm_resource_list { UINT32 ResourceCount; - ACPI_DRTM_RESOURCE Resources[1]; + ACPI_DRTM_RESOURCE Resources[]; } ACPI_DRTM_RESOURCE_LIST; @@ -1319,7 +1319,7 @@ typedef struct acpi_table_ecdt ACPI_GENERIC_ADDRESS Data; /* Address of EC data register */ UINT32 Uid; /* Unique ID - must be same as the EC _UID method */ UINT8 Gpe; /* The GPE for the EC */ - UINT8 Id[1]; /* Full namepath of the EC in the ACPI namespace */ + UINT8 Id[]; /* Full namepath of the EC in the ACPI namespace */ } ACPI_TABLE_ECDT; -- cgit v1.2.1 From 44f1af0664599e87bebc3a1260692baa27b2f264 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 23 Feb 2023 17:12:21 -0800 Subject: actbl2: Replace 1-element arrays with flexible arrays Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). The sizeof() uses with ACPI_NFIT_FLUSH_ADDRESS and ACPI_NFIT_SMBIOS have been adjusted to drop the open-coded subtraction of the trailing single element. The result is no binary differences in .text nor .data sections. --- source/common/dmtbdump2.c | 6 +++--- source/include/actbl2.h | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/source/common/dmtbdump2.c b/source/common/dmtbdump2.c index 353979e8f..7216f877f 100644 --- a/source/common/dmtbdump2.c +++ b/source/common/dmtbdump2.c @@ -1538,7 +1538,7 @@ AcpiDmDumpNfit ( /* Has a variable number of 64-bit addresses at the end */ InfoTable = AcpiDmTableInfoNfit6; - FieldOffset = sizeof (ACPI_NFIT_FLUSH_ADDRESS) - sizeof (UINT64); + FieldOffset = sizeof (ACPI_NFIT_FLUSH_ADDRESS); break; case ACPI_NFIT_TYPE_CAPABILITIES: /* ACPI 6.0A */ @@ -1592,12 +1592,12 @@ AcpiDmDumpNfit ( case ACPI_NFIT_TYPE_SMBIOS: Length = Subtable->Length - - sizeof (ACPI_NFIT_SMBIOS) + sizeof (UINT8); + sizeof (ACPI_NFIT_SMBIOS); if (Length) { Status = AcpiDmDumpTable (Table->Length, - sizeof (ACPI_NFIT_SMBIOS) - sizeof (UINT8), + sizeof (ACPI_NFIT_SMBIOS), SmbiosInfo, Length, AcpiDmTableInfoNfit3a); if (ACPI_FAILURE (Status)) diff --git a/source/include/actbl2.h b/source/include/actbl2.h index 6dc2efd0c..6f13261a6 100644 --- a/source/include/actbl2.h +++ b/source/include/actbl2.h @@ -572,7 +572,7 @@ typedef struct acpi_iort_node UINT32 Identifier; UINT32 MappingCount; UINT32 MappingOffset; - char NodeData[1]; + char NodeData[]; } ACPI_IORT_NODE; @@ -638,7 +638,7 @@ typedef struct acpi_iort_memory_access typedef struct acpi_iort_its_group { UINT32 ItsCount; - UINT32 Identifiers[1]; /* GIC ITS identifier array */ + UINT32 Identifiers[]; /* GIC ITS identifier array */ } ACPI_IORT_ITS_GROUP; @@ -648,7 +648,7 @@ typedef struct acpi_iort_named_component UINT32 NodeFlags; UINT64 MemoryProperties; /* Memory access properties */ UINT8 MemoryAddressLimit; /* Memory address size limit */ - char DeviceName[1]; /* Path of namespace object */ + char DeviceName[]; /* Path of namespace object */ } ACPI_IORT_NAMED_COMPONENT; @@ -664,7 +664,7 @@ typedef struct acpi_iort_root_complex UINT32 PciSegmentNumber; UINT8 MemoryAddressLimit; /* Memory address size limit */ UINT16 PasidCapabilities; /* PASID Capabilities */ - UINT8 Reserved[1]; /* Reserved, must be zero */ + UINT8 Reserved[]; /* Reserved, must be zero */ } ACPI_IORT_ROOT_COMPLEX; @@ -688,7 +688,7 @@ typedef struct acpi_iort_smmu UINT32 ContextInterruptOffset; UINT32 PmuInterruptCount; UINT32 PmuInterruptOffset; - UINT64 Interrupts[1]; /* Interrupt array */ + UINT64 Interrupts[]; /* Interrupt array */ } ACPI_IORT_SMMU; @@ -1240,7 +1240,7 @@ typedef struct acpi_madt_local_sapic UINT8 Reserved[3]; /* Reserved, must be zero */ UINT32 LapicFlags; UINT32 Uid; /* Numeric UID - ACPI 3.0 */ - char UidString[1]; /* String UID - ACPI 3.0 */ + char UidString[]; /* String UID - ACPI 3.0 */ } ACPI_MADT_LOCAL_SAPIC; @@ -2056,7 +2056,7 @@ typedef struct acpi_nfit_smbios { ACPI_NFIT_HEADER Header; UINT32 Reserved; /* Reserved, must be zero */ - UINT8 Data[1]; /* Variable length */ + UINT8 Data[]; /* Variable length */ } ACPI_NFIT_SMBIOS; @@ -2122,7 +2122,7 @@ typedef struct acpi_nfit_flush_address UINT32 DeviceHandle; UINT16 HintCount; UINT8 Reserved[6]; /* Reserved, must be zero */ - UINT64 HintAddress[1]; /* Variable length */ + UINT64 HintAddress[]; /* Variable length */ } ACPI_NFIT_FLUSH_ADDRESS; -- cgit v1.2.1 From e66decc6fca36b59194b0947d87d6a9bec078bc3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 1 Mar 2023 14:43:22 -0800 Subject: ACPI_NFIT_INTERLEAVE: Replace 1-element array with flexible array Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). Unlike ACPI_NFIT_FLUSH_ADDRESS and ACPI_NFIT_SMBIOS, which had their sizeof() uses adjusted in code, ACPI_NFIT_INTERLEAVE did not. This appears to have been a bug. After this change, there is a binary difference in AcpiDmDumpNfit() since the size of the structure now has the correct size, as the prior result was including the trailing U32: - mov $0x14,%ebp + mov $0x10,%ebp Signed-off-by: Kees Cook --- source/include/actbl2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/include/actbl2.h b/source/include/actbl2.h index 6f13261a6..c594658ac 100644 --- a/source/include/actbl2.h +++ b/source/include/actbl2.h @@ -2045,7 +2045,7 @@ typedef struct acpi_nfit_interleave UINT16 Reserved; /* Reserved, must be zero */ UINT32 LineCount; UINT32 LineSize; - UINT32 LineOffset[1]; /* Variable length */ + UINT32 LineOffset[]; /* Variable length */ } ACPI_NFIT_INTERLEAVE; -- cgit v1.2.1 From e73b227e8e475c20cc394f237ea35d592fdf9ec3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 18 Nov 2022 09:48:08 -0800 Subject: Introduce ACPI_FLEX_ARRAY In order to enable using -fstrict-flex-arrays with GCC and Clang in the Linux kernel, each trailing dynamically sized array must be defined as proper C99 "flexible array members" (FAM). Unfortunately, ACPICA has a bunch of technical debt, dating back to before even the GNU extension of 0-length arrays, meaning the code base has many 1-element and 0-length arrays defined at the end of structures that should actually be FAMs. One limitation of the C99 FAM specification is the accidental requirement that they cannot be in unions or alone in structs. There is no real-world reason for this, though, and, actually, the existing GNU extension permits this for 0-length arrays (which get treated as FAMs). Add the ACPI_FLEX_ARRAY() helper macro to work around this requirement so that FAMs can be defined in unions or alone in structs. Since this behavior still depends on GNU extensions, keep the macro specific to GCC (and Clang) builds. In this way, MSVC will continue to use 0-length arrays (since it does not support the union work-around). When MSVC grows support for this in the future, the macro can be updated. --- source/include/actypes.h | 4 ++++ source/include/platform/acgcc.h | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/source/include/actypes.h b/source/include/actypes.h index 52d2d6dcf..c0f0a87f4 100644 --- a/source/include/actypes.h +++ b/source/include/actypes.h @@ -1574,4 +1574,8 @@ typedef enum #define ACPI_FALLTHROUGH do {} while(0) #endif +#ifndef ACPI_FLEX_ARRAY +#define ACPI_FLEX_ARRAY(TYPE, NAME) TYPE NAME[0] +#endif + #endif /* __ACTYPES_H__ */ diff --git a/source/include/platform/acgcc.h b/source/include/platform/acgcc.h index 81e29f90d..55a66c6da 100644 --- a/source/include/platform/acgcc.h +++ b/source/include/platform/acgcc.h @@ -211,4 +211,15 @@ typedef __builtin_va_list va_list; #define ACPI_FALLTHROUGH __attribute__((__fallthrough__)) #endif +/* + * Flexible array members are not allowed to be part of a union under + * C99, but this is not for any technical reason. Work around the + * limitation. + */ +#define ACPI_FLEX_ARRAY(TYPE, NAME) \ + struct { \ + struct { } __Empty_ ## NAME; \ + TYPE NAME[]; \ + } + #endif /* __ACGCC_H__ */ -- cgit v1.2.1 From 8409bb869a1790f6e02391c3f0eaf9c5fa63e33f Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 23 Feb 2023 17:12:21 -0800 Subject: ACPI_RESOURCE_DMA: Replace 1-element array with flexible array Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99, but without changing the structure size. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). As with IRQs, leave a single element in a union. No binary changes appear in the .text nor .data sections. --- source/include/acrestyp.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/include/acrestyp.h b/source/include/acrestyp.h index 16356098f..252cebc1b 100644 --- a/source/include/acrestyp.h +++ b/source/include/acrestyp.h @@ -300,7 +300,10 @@ typedef struct acpi_resource_dma UINT8 BusMaster; UINT8 Transfer; UINT8 ChannelCount; - UINT8 Channels[1]; + union { + UINT8 Channel; + ACPI_FLEX_ARRAY(UINT8, Channels); + }; } ACPI_RESOURCE_DMA; -- cgit v1.2.1 From f4a3afd78c28dede0907f47951f0b73c9a776d4e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 17 Nov 2022 20:42:06 -0800 Subject: ACPI_PCI_ROUTING_TABLE: Replace fixed-size array with flex array member The "Source" array is actually a dynamically sized array, but it is defined as a fixed-size 4 byte array. This results in tripping both compile-time and run-time bounds checkers (e.g. via either __builtin_object_size() or -fsanitize=bounds). To retain the padding, create a union with an unused Pad variable of size 4, and redefine Source as a proper flexible array member. No binary changes appear in the .text nor .data sections. --- source/include/acrestyp.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source/include/acrestyp.h b/source/include/acrestyp.h index 252cebc1b..ccde5bdde 100644 --- a/source/include/acrestyp.h +++ b/source/include/acrestyp.h @@ -942,8 +942,10 @@ typedef struct acpi_pci_routing_table UINT32 Pin; UINT64 Address; /* here for 64-bit alignment */ UINT32 SourceIndex; - 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 */ + ACPI_FLEX_ARRAY(char, Source); + }; } ACPI_PCI_ROUTING_TABLE; #endif /* __ACRESTYP_H__ */ -- cgit v1.2.1 From 3c19ae70424e9ab1e1b805203d300d2660f9a2f7 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 1 Mar 2023 14:42:10 -0800 Subject: ACPI_DMAR_ANDD: Replace 1-element array with flexible array Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). The handling of ACPI_DMAR_ANDD by AcpiDmDumpDmar() appears to expect a single trailing char for calculating table offsets. Keep a char in the union to avoid any code changes appearing in the .text or .data sections. Signed-off-by: Kees Cook --- source/include/actbl1.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/include/actbl1.h b/source/include/actbl1.h index 63f137117..cecd7edbe 100644 --- a/source/include/actbl1.h +++ b/source/include/actbl1.h @@ -1219,7 +1219,10 @@ typedef struct acpi_dmar_andd ACPI_DMAR_HEADER Header; UINT8 Reserved[3]; UINT8 DeviceNumber; - char DeviceName[1]; + union { + char __pad; + ACPI_FLEX_ARRAY(char, DeviceName); + }; } ACPI_DMAR_ANDD; -- cgit v1.2.1 From e7f6d8c1b7f79eb4b9b07f1bc09c549a2acbd6e8 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 18 Nov 2022 09:49:35 -0800 Subject: ACPI_MADT_OEM_DATA: Fix flexible array member definition Use ACPI_FLEX_ARRAY() helper to define flexible array member alone in a struct. Fixes issue #812. No binary changes appear in the .text nor .data sections. --- source/include/actbl2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/include/actbl2.h b/source/include/actbl2.h index c594658ac..7c3243385 100644 --- a/source/include/actbl2.h +++ b/source/include/actbl2.h @@ -1552,7 +1552,7 @@ enum AcpiMadtLpcPicVersion { typedef struct acpi_madt_oem_data { - UINT8 OemData[0]; + ACPI_FLEX_ARRAY(UINT8, OemData); } ACPI_MADT_OEM_DATA; -- cgit v1.2.1 From 9879d9995482be4c76d56d41d4074dd3146c690f Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 23 Feb 2023 17:12:21 -0800 Subject: ASL_CACHE_INFO: Replace 1-element array with flexible array Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). One binary difference appears in aslcache.o, which appears to be the compiler choosing different working registers. No other changes are seen in the .text nor .data sections. --- source/compiler/asltypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/compiler/asltypes.h b/source/compiler/asltypes.h index 30cd7b789..577d3b881 100644 --- a/source/compiler/asltypes.h +++ b/source/compiler/asltypes.h @@ -336,7 +336,7 @@ typedef struct asl_file_desc typedef struct asl_cache_info { void *Next; - char Buffer[1]; + char Buffer[]; } ASL_CACHE_INFO; -- cgit v1.2.1 From bfdd3446e7caf795c85c70326c137023942972c5 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 17 Nov 2022 20:48:16 -0800 Subject: ACPI_RESOURCE_IRQ: Replace 1-element arrays with flexible array Similar to commit 7ba2f3d91a32 ("Replace one-element array with flexible-array"), replace the 1-element array with a proper flexible array member as defined by C99. This allows the code to operate without tripping compile-time and run-time bounds checkers (e.g. via __builtin_object_size(), -fsanitize=bounds, and/or -fstrict-flex-arrays=3). Note that the spec requires there be at least one interrupt, so use a union to keep space allocated for this. The only binary change in .text and .data sections is some rearrangement by the compiler of AcpiDmAddressCommon(), but appears to be harmless. --- source/include/acrestyp.h | 10 ++++++++-- source/include/amlresrc.h | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/source/include/acrestyp.h b/source/include/acrestyp.h index ccde5bdde..0dab6ec7b 100644 --- a/source/include/acrestyp.h +++ b/source/include/acrestyp.h @@ -290,7 +290,10 @@ typedef struct acpi_resource_irq UINT8 Shareable; UINT8 WakeCapable; UINT8 InterruptCount; - UINT8 Interrupts[1]; + union { + UINT8 Interrupt; + ACPI_FLEX_ARRAY(UINT8, Interrupts); + }; } ACPI_RESOURCE_IRQ; @@ -541,7 +544,10 @@ typedef struct acpi_resource_extended_irq UINT8 WakeCapable; UINT8 InterruptCount; ACPI_RESOURCE_SOURCE ResourceSource; - UINT32 Interrupts[1]; + union { + UINT32 Interrupt; + ACPI_FLEX_ARRAY(UINT32, Interrupts); + }; } ACPI_RESOURCE_EXTENDED_IRQ; diff --git a/source/include/amlresrc.h b/source/include/amlresrc.h index 1ad70d171..05fa1505d 100644 --- a/source/include/amlresrc.h +++ b/source/include/amlresrc.h @@ -503,7 +503,10 @@ typedef struct aml_resource_extended_irq AML_RESOURCE_LARGE_HEADER_COMMON UINT8 Flags; UINT8 InterruptCount; - UINT32 Interrupts[1]; + union { + UINT32 Interrupt; + ACPI_FLEX_ARRAY(UINT32, Interrupts); + }; /* ResSourceIndex, ResSource optional fields follow */ } AML_RESOURCE_EXTENDED_IRQ; -- cgit v1.2.1