Skip to content

Commit 1c28da2

Browse files
tamirdtamird
committed
Apply pack(1) to AML_RESOURCE
This was originally done in NetBSD: NetBSD/src@b69d1ac and is the correct alternative to the smattering of `memcpy`s I previously contributed to this repository. This also sidesteps the newly strict checks added in UBSAN: llvm/llvm-project@7926744 Before this change we see the following UBSAN stack trace in Fuchsia: #0 0x000021afcfdeca5e in AcpiRsGetAddressCommon(ACPI_RESOURCE*, AML_RESOURCE*) ../../third_party/acpica/source/components/resources/rsaddr.c:329 <platform-bus-x86.so>+0x6aca5e #1.2 0x000021982bc4af3c in ubsan_GetStackTrace() compiler-rt/lib/ubsan/ubsan_diag.cpp:41 <libclang_rt.asan.so>+0x41f3c #1.1 0x000021982bc4af3c in MaybePrintStackTrace() compiler-rt/lib/ubsan/ubsan_diag.cpp:51 <libclang_rt.asan.so>+0x41f3c #1 0x000021982bc4af3c in ~ScopedReport() compiler-rt/lib/ubsan/ubsan_diag.cpp:395 <libclang_rt.asan.so>+0x41f3c #2 0x000021982bc4bb6f in handleTypeMismatchImpl() compiler-rt/lib/ubsan/ubsan_handlers.cpp:137 <libclang_rt.asan.so>+0x42b6f #3 0x000021982bc4b723 in __ubsan_handle_type_mismatch_v1 compiler-rt/lib/ubsan/ubsan_handlers.cpp:142 <libclang_rt.asan.so>+0x42723 #4 0x000021afcfdeca5e in AcpiRsGetAddressCommon(ACPI_RESOURCE*, AML_RESOURCE*) ../../third_party/acpica/source/components/resources/rsaddr.c:329 <platform-bus-x86.so>+0x6aca5e #5 0x000021afcfdf2089 in AcpiRsConvertAmlToResource(ACPI_RESOURCE*, AML_RESOURCE*, ACPI_RSCONVERT_INFO*) ../../third_party/acpica/source/components/resources/rsmisc.c:355 <platform-bus-x86.so>+0x6b2089 #6 0x000021afcfded169 in AcpiRsConvertAmlToResources(UINT8*, UINT32, UINT32, UINT8, void**) ../../third_party/acpica/source/components/resources/rslist.c:137 <platform-bus-x86.so>+0x6ad169 #7 0x000021afcfe2d24a in AcpiUtWalkAmlResources(ACPI_WALK_STATE*, UINT8*, ACPI_SIZE, ACPI_WALK_AML_CALLBACK, void**) ../../third_party/acpica/source/components/utilities/utresrc.c:237 <platform-bus-x86.so>+0x6ed24a #8 0x000021afcfde66b7 in AcpiRsCreateResourceList(ACPI_OPERAND_OBJECT*, ACPI_BUFFER*) ../../third_party/acpica/source/components/resources/rscreate.c:199 <platform-bus-x86.so>+0x6a66b7 #9 0x000021afcfdf6979 in AcpiRsGetMethodData(ACPI_HANDLE, const char*, ACPI_BUFFER*) ../../third_party/acpica/source/components/resources/rsutils.c:770 <platform-bus-x86.so>+0x6b6979 #10 0x000021afcfdf708f in AcpiWalkResources(ACPI_HANDLE, char*, ACPI_WALK_RESOURCE_CALLBACK, void*) ../../third_party/acpica/source/components/resources/rsxface.c:731 <platform-bus-x86.so>+0x6b708f #11 0x000021afcfa95dcf in acpi::AcpiImpl::WalkResources(acpi::AcpiImpl*, ACPI_HANDLE, const char*, acpi::Acpi::ResourcesCallable) ../../src/devices/board/lib/acpi/acpi-impl.cc:41 <platform-bus-x86.so>+0x355dcf #12 0x000021afcfaa8278 in acpi::DeviceBuilder::GatherResources(acpi::DeviceBuilder*, acpi::Acpi*, fidl::AnyArena&, acpi::Manager*, acpi::DeviceBuilder::GatherResourcesCallback) ../../src/devices/board/lib/acpi/device-builder.cc:84 <platform-bus-x86.so>+0x368278 #13 0x000021afcfbddb87 in acpi::Manager::ConfigureDiscoveredDevices(acpi::Manager*) ../../src/devices/board/lib/acpi/manager.cc:75 <platform-bus-x86.so>+0x49db87 #14 0x000021afcf99091d in publish_acpi_devices(acpi::Manager*, zx_device_t*, zx_device_t*) ../../src/devices/board/drivers/x86/acpi-nswalk.cc:95 <platform-bus-x86.so>+0x25091d #15 0x000021afcf9c1d4e in x86::X86::DoInit(x86::X86*) ../../src/devices/board/drivers/x86/x86.cc:60 <platform-bus-x86.so>+0x281d4e #16 0x000021afcf9e33ad in λ(x86::X86::DdkInit::(anon class)*) ../../src/devices/board/drivers/x86/x86.cc:77 <platform-bus-x86.so>+0x2a33ad #17 0x000021afcf9e313e in fit::internal::target<(lambda at../../src/devices/board/drivers/x86/x86.cc:76:19), false, false, std::__2::allocator<std::byte>, void>::invoke(void*) ../../sdk/lib/fit/include/lib/fit/internal/function.h:183 <platform-bus-x86.so>+0x2a313e #18 0x000021afcfbab4c7 in fit::internal::function_base<16UL, false, void(), std::__2::allocator<std::byte>>::invoke(const fit::internal::function_base<16UL, false, void (), std::__2::allocator<std::byte> >*) ../../sdk/lib/fit/include/lib/fit/internal/function.h:522 <platform-bus-x86.so>+0x46b4c7 #19 0x000021afcfbab342 in fit::function_impl<16UL, false, void(), std::__2::allocator<std::byte>>::operator()(const fit::function_impl<16UL, false, void (), std::__2::allocator<std::byte> >*) ../../sdk/lib/fit/include/lib/fit/function.h:315 <platform-bus-x86.so>+0x46b342 #20 0x000021afcfcd98c3 in async::internal::RetainedTask::Handler(async_dispatcher_t*, async_task_t*, zx_status_t) ../../sdk/lib/async/task.cc:24 <platform-bus-x86.so>+0x5998c3 #21 0x00002290f9924616 in λ(const driver_runtime::Dispatcher::PostTask::(anon class)*, std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >, zx_status_t) ../../src/devices/bin/driver_runtime/dispatcher.cc:789 <libdriver_runtime.so>+0x10a616 #22 0x00002290f9924323 in fit::internal::target<(lambda at../../src/devices/bin/driver_runtime/dispatcher.cc:788:7), true, false, std::__2::allocator<std::byte>, void, std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest>>, int>::invoke(void*, std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >, int) ../../sdk/lib/fit/include/lib/fit/internal/function.h:128 <libdriver_runtime.so>+0x10a323 #23 0x00002290f9904b76 in fit::internal::function_base<24UL, true, void(std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest>>, int), std::__2::allocator<std::byte>>::invoke(const fit::internal::function_base<24UL, true, void (std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >, int), std::__2::allocator<std::byte> >*, std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >, int) ../../sdk/lib/fit/include/lib/fit/internal/function.h:522 <libdriver_runtime.so>+0xeab76 #24 0x00002290f9904831 in fit::callback_impl<24UL, true, void(std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest>>, int), std::__2::allocator<std::byte>>::operator()(fit::callback_impl<24UL, true, void (std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >, int), std::__2::allocator<std::byte> >*, std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >, int) ../../sdk/lib/fit/include/lib/fit/function.h:471 <libdriver_runtime.so>+0xea831 #25 0x00002290f98d5adc in driver_runtime::CallbackRequest::Call(driver_runtime::CallbackRequest*, std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >, zx_status_t) ../../src/devices/bin/driver_runtime/callback_request.h:74 <libdriver_runtime.so>+0xbbadc #26 0x00002290f98e1e58 in driver_runtime::Dispatcher::DispatchCallback(driver_runtime::Dispatcher*, std::__2::unique_ptr<driver_runtime::CallbackRequest, std::__2::default_delete<driver_runtime::CallbackRequest> >) ../../src/devices/bin/driver_runtime/dispatcher.cc:1248 <libdriver_runtime.so>+0xc7e58 #27 0x00002290f98e4159 in driver_runtime::Dispatcher::DispatchCallbacks(driver_runtime::Dispatcher*, std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>) ../../src/devices/bin/driver_runtime/dispatcher.cc:1308 <libdriver_runtime.so>+0xca159 #28 0x00002290f9918414 in λ(const driver_runtime::Dispatcher::CreateWithAdder::(anon class)*, std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>) ../../src/devices/bin/driver_runtime/dispatcher.cc:353 <libdriver_runtime.so>+0xfe414 #29 0x00002290f991812d in fit::internal::target<(lambda at../../src/devices/bin/driver_runtime/dispatcher.cc:351:7), true, false, std::__2::allocator<std::byte>, void, std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter>>, fbl::RefPtr<driver_runtime::Dispatcher>>::invoke(void*, std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>) ../../sdk/lib/fit/include/lib/fit/internal/function.h:128 <libdriver_runtime.so>+0xfe12d #30 0x00002290f9906fc7 in fit::internal::function_base<8UL, true, void(std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter>>, fbl::RefPtr<driver_runtime::Dispatcher>), std::__2::allocator<std::byte>>::invoke(const fit::internal::function_base<8UL, true, void (std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>), std::__2::allocator<std::byte> >*, std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>) ../../sdk/lib/fit/include/lib/fit/internal/function.h:522 <libdriver_runtime.so>+0xecfc7 #31 0x00002290f9906c66 in fit::function_impl<8UL, true, void(std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter>>, fbl::RefPtr<driver_runtime::Dispatcher>), std::__2::allocator<std::byte>>::operator()(const fit::function_impl<8UL, true, void (std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>), std::__2::allocator<std::byte> >*, std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>) ../../sdk/lib/fit/include/lib/fit/function.h:315 <libdriver_runtime.so>+0xecc66 #32 0x00002290f98e73d9 in driver_runtime::Dispatcher::EventWaiter::InvokeCallback(driver_runtime::Dispatcher::EventWaiter*, std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, fbl::RefPtr<driver_runtime::Dispatcher>) ../../src/devices/bin/driver_runtime/dispatcher.h:543 <libdriver_runtime.so>+0xcd3d9 #33 0x00002290f98e700d in driver_runtime::Dispatcher::EventWaiter::HandleEvent(std::__2::unique_ptr<driver_runtime::Dispatcher::EventWaiter, std::__2::default_delete<driver_runtime::Dispatcher::EventWaiter> >, async_dispatcher_t*, async::WaitBase*, zx_status_t, zx_packet_signal_t const*) ../../src/devices/bin/driver_runtime/dispatcher.cc:1442 <libdriver_runtime.so>+0xcd00d #34 0x00002290f9918983 in AsyncLoopOwnedEventHandler<driver_runtime::Dispatcher::EventWaiter>::HandleEvent(AsyncLoopOwnedEventHandler<driver_runtime::Dispatcher::EventWaiter>*, async_dispatcher_t*, async::WaitBase*, zx_status_t, zx_packet_signal_t const*) ../../src/devices/bin/driver_runtime/async_loop_owned_event_handler.h:59 <libdriver_runtime.so>+0xfe983 #35 0x00002290f9918b9e in async::WaitMethod<AsyncLoopOwnedEventHandler<driver_runtime::Dispatcher::EventWaiter>, &AsyncLoopOwnedEventHandler<driver_runtime::Dispatcher::EventWaiter>::HandleEvent>::CallHandler(async_dispatcher_t*, async_wait_t*, zx_status_t, zx_packet_signal_t const*) ../../sdk/lib/async/include/lib/async/cpp/wait.h:201 <libdriver_runtime.so>+0xfeb9e #36 0x00002290f99bf509 in async_loop_dispatch_wait(async_loop_t*, async_wait_t*, zx_status_t, zx_packet_signal_t const*) ../../sdk/lib/async-loop/loop.c:394 <libdriver_runtime.so>+0x1a5509 #37 0x00002290f99b9958 in async_loop_run_once(async_loop_t*, zx_time_t) ../../sdk/lib/async-loop/loop.c:343 <libdriver_runtime.so>+0x19f958 #38 0x00002290f99b9247 in async_loop_run(async_loop_t*, zx_time_t, _Bool) ../../sdk/lib/async-loop/loop.c:301 <libdriver_runtime.so>+0x19f247 #39 0x00002290f99ba962 in async_loop_run_thread(void*) ../../sdk/lib/async-loop/loop.c:860 <libdriver_runtime.so>+0x1a0962 #40 0x000041afd176ef30 in start_c11(void*) ../../zircon/third_party/ulib/musl/pthread/pthread_create.c:63 <libc.so>+0x84f30 #41 0x000041afd18a448d in thread_trampoline(uintptr_t, uintptr_t) ../../zircon/system/ulib/runtime/thread.cc:100 <libc.so>+0x1ba48d
1 parent fd6e4b3 commit 1c28da2

File tree

5 files changed

+15
-41
lines changed

5 files changed

+15
-41
lines changed

‎source/components/resources/rsaddr.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -431,18 +431,13 @@ AcpiRsGetAddressCommon (
431431
ACPI_RESOURCE *Resource,
432432
AML_RESOURCE *Aml)
433433
{
434-
435-
/* Avoid undefined behavior: member access within misaligned address */
436-
437-
AML_RESOURCE_ADDRESS Address;
438-
memcpy(&Address, Aml, sizeof(Address));
439434
ACPI_FUNCTION_ENTRY();
440435

441436
/* Validate the Resource Type */
442437

443-
if ((Address.ResourceType > 2) &&
444-
(Address.ResourceType < 0xC0) &&
445-
(Address.ResourceType != 0x0A))
438+
if ((Aml->Address.ResourceType > 2) &&
439+
(Aml->Address.ResourceType < 0xC0) &&
440+
(Aml->Address.ResourceType != 0x0A))
446441
{
447442
return (FALSE);
448443
}
@@ -469,7 +464,7 @@ AcpiRsGetAddressCommon (
469464
/* Generic resource type, just grab the TypeSpecific byte */
470465

471466
Resource->Data.Address.Info.TypeSpecific =
472-
Address.SpecificFlags;
467+
Aml->Address.SpecificFlags;
473468
}
474469

475470
return (TRUE);
@@ -497,7 +492,6 @@ AcpiRsSetAddressCommon (
497492
{
498493
ACPI_FUNCTION_ENTRY ();
499494

500-
501495
/* Set the Resource Type and General Flags */
502496

503497
(void) AcpiRsConvertResourceToAml (

‎source/components/resources/rscalc.c

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -741,15 +741,11 @@ AcpiRsGetListLength (
741741
break;
742742

743743
case ACPI_RESOURCE_NAME_SERIAL_BUS: {
744-
/* Avoid undefined behavior: member access within misaligned address */
745-
746-
AML_RESOURCE_COMMON_SERIALBUS CommonSerialBus;
747-
memcpy(&CommonSerialBus, AmlResource, sizeof(CommonSerialBus));
748744

749745
MinimumAmlResourceLength = AcpiGbl_ResourceAmlSerialBusSizes[
750-
CommonSerialBus.Type];
746+
AmlResource->CommonSerialBus.Type];
751747
ExtraStructBytes +=
752-
CommonSerialBus.ResourceLength -
748+
AmlResource->CommonSerialBus.ResourceLength -
753749
MinimumAmlResourceLength;
754750
break;
755751
}
@@ -821,13 +817,8 @@ AcpiRsGetListLength (
821817
if (AcpiUtGetResourceType (AmlBuffer) ==
822818
ACPI_RESOURCE_NAME_SERIAL_BUS)
823819
{
824-
/* Avoid undefined behavior: member access within misaligned address */
825-
826-
AML_RESOURCE_COMMON_SERIALBUS CommonSerialBus;
827-
memcpy(&CommonSerialBus, AmlResource, sizeof(CommonSerialBus));
828-
829820
BufferSize = AcpiGbl_ResourceStructSerialBusSizes[
830-
CommonSerialBus.Type] + ExtraStructBytes;
821+
AmlResource->CommonSerialBus.Type] + ExtraStructBytes;
831822
}
832823
else
833824
{

‎source/components/resources/rslist.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,7 @@ AcpiRsConvertAmlToResources (
209209
if (AcpiUtGetResourceType (Aml) ==
210210
ACPI_RESOURCE_NAME_SERIAL_BUS)
211211
{
212-
/* Avoid undefined behavior: member access within misaligned address */
213-
214-
AML_RESOURCE_COMMON_SERIALBUS CommonSerialBus;
215-
memcpy(&CommonSerialBus, AmlResource, sizeof(CommonSerialBus));
216-
217-
if (CommonSerialBus.Type >
212+
if (AmlResource->CommonSerialBus.Type >
218213
AML_RESOURCE_MAX_SERIALBUSTYPE)
219214
{
220215
ConversionTable = NULL;
@@ -224,7 +219,7 @@ AcpiRsConvertAmlToResources (
224219
/* This is an I2C, SPI, UART, or CSI2 SerialBus descriptor */
225220

226221
ConversionTable = AcpiGbl_ConvertResourceSerialBusDispatch [
227-
CommonSerialBus.Type];
222+
AmlResource->CommonSerialBus.Type];
228223
}
229224
}
230225
else

‎source/components/utilities/utresrc.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -532,21 +532,16 @@ AcpiUtValidateResource (
532532
AmlResource = ACPI_CAST_PTR (AML_RESOURCE, Aml);
533533
if (ResourceType == ACPI_RESOURCE_NAME_SERIAL_BUS)
534534
{
535-
/* Avoid undefined behavior: member access within misaligned address */
536-
537-
AML_RESOURCE_COMMON_SERIALBUS CommonSerialBus;
538-
memcpy(&CommonSerialBus, AmlResource, sizeof(CommonSerialBus));
539-
540535
/* Validate the BusType field */
541536

542-
if ((CommonSerialBus.Type == 0) ||
543-
(CommonSerialBus.Type > AML_RESOURCE_MAX_SERIALBUSTYPE))
537+
if ((AmlResource->CommonSerialBus.Type == 0) ||
538+
(AmlResource->CommonSerialBus.Type > AML_RESOURCE_MAX_SERIALBUSTYPE))
544539
{
545540
if (WalkState)
546541
{
547542
ACPI_ERROR ((AE_INFO,
548543
"Invalid/unsupported SerialBus resource descriptor: BusType 0x%2.2X",
549-
CommonSerialBus.Type));
544+
AmlResource->CommonSerialBus.Type));
550545
}
551546
return (AE_AML_INVALID_RESOURCE_TYPE);
552547
}

‎source/include/amlresrc.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -792,10 +792,6 @@ typedef struct aml_resource_pin_group_config
792792

793793
#define AML_RESOURCE_PIN_GROUP_CONFIG_REVISION 1 /* ACPI 6.2 */
794794

795-
/* restore default alignment */
796-
797-
#pragma pack()
798-
799795
/* Union of all resource descriptors, so we can allocate the worst case */
800796

801797
typedef union aml_resource
@@ -852,6 +848,9 @@ typedef union aml_resource
852848

853849
} AML_RESOURCE;
854850

851+
/* restore default alignment */
852+
853+
#pragma pack()
855854

856855
/* Interfaces used by both the disassembler and compiler */
857856

0 commit comments

Comments
 (0)