diff options
| author | Tamir Duberstein <tamird@google.com> | 2023-02-24 22:28:28 -0500 |
|---|---|---|
| committer | Tamir Duberstein <tamird@google.com> | 2023-02-27 13:56:21 -0500 |
| commit | 770653e3ba67c30a629ca7d12e352d83c2541b1e (patch) | |
| tree | e31ce4acce305fb4fe4eea2376ff5cc6eedbc636 | |
| parent | 682350c40c13504e3e880ebfd7d08b9f803d72dd (diff) | |
| download | acpica-770653e3ba67c30a629ca7d12e352d83c2541b1e.tar.gz | |
Avoid undefined behavior: applying zero offset to null pointer
Before this change we see the following UBSAN stack trace in Fuchsia:
#0 0x000021e4213b3302 in AcpiDsInitAmlWalk(ACPI_WALK_STATE*, ACPI_PARSE_OBJECT*, ACPI_NAMESPACE_NODE*, UINT8*, UINT32, ACPI_EVALUATE_INFO*, UINT8) ../../third_party/acpica/source/components/dispatcher/dswstate.c:682 <platform-bus-x86.so>+0x233302
#1.2 0x000020d0f660777f in ubsan_GetStackTrace() compiler-rt/lib/ubsan/ubsan_diag.cpp:41 <libclang_rt.asan.so>+0x3d77f
#1.1 0x000020d0f660777f in MaybePrintStackTrace() compiler-rt/lib/ubsan/ubsan_diag.cpp:51 <libclang_rt.asan.so>+0x3d77f
#1 0x000020d0f660777f in ~ScopedReport() compiler-rt/lib/ubsan/ubsan_diag.cpp:387 <libclang_rt.asan.so>+0x3d77f
#2 0x000020d0f660b96d in handlePointerOverflowImpl() compiler-rt/lib/ubsan/ubsan_handlers.cpp:809 <libclang_rt.asan.so>+0x4196d
#3 0x000020d0f660b50d in compiler-rt/lib/ubsan/ubsan_handlers.cpp:815 <libclang_rt.asan.so>+0x4150d
#4 0x000021e4213b3302 in AcpiDsInitAmlWalk(ACPI_WALK_STATE*, ACPI_PARSE_OBJECT*, ACPI_NAMESPACE_NODE*, UINT8*, UINT32, ACPI_EVALUATE_INFO*, UINT8) ../../third_party/acpica/source/components/dispatcher/dswstate.c:682 <platform-bus-x86.so>+0x233302
#5 0x000021e4213e2369 in AcpiDsCallControlMethod(ACPI_THREAD_STATE*, ACPI_WALK_STATE*, ACPI_PARSE_OBJECT*) ../../third_party/acpica/source/components/dispatcher/dsmethod.c:605 <platform-bus-x86.so>+0x262369
#6 0x000021e421437fac in AcpiPsParseAml(ACPI_WALK_STATE*) ../../third_party/acpica/source/components/parser/psparse.c:550 <platform-bus-x86.so>+0x2b7fac
#7 0x000021e4214464d2 in AcpiPsExecuteMethod(ACPI_EVALUATE_INFO*) ../../third_party/acpica/source/components/parser/psxface.c:244 <platform-bus-x86.so>+0x2c64d2
#8 0x000021e4213aa052 in AcpiNsEvaluate(ACPI_EVALUATE_INFO*) ../../third_party/acpica/source/components/namespace/nseval.c:250 <platform-bus-x86.so>+0x22a052
#9 0x000021e421413dd8 in AcpiNsInitOneDevice(ACPI_HANDLE, UINT32, void*, void**) ../../third_party/acpica/source/components/namespace/nsinit.c:735 <platform-bus-x86.so>+0x293dd8
#10 0x000021e421429e98 in AcpiNsWalkNamespace(ACPI_OBJECT_TYPE, ACPI_HANDLE, UINT32, UINT32, ACPI_WALK_CALLBACK, ACPI_WALK_CALLBACK, void*, void**) ../../third_party/acpica/source/components/namespace/nswalk.c:298 <platform-bus-x86.so>+0x2a9e98
#11 0x000021e4214131ac in AcpiNsInitializeDevices(UINT32) ../../third_party/acpica/source/components/namespace/nsinit.c:268 <platform-bus-x86.so>+0x2931ac
#12 0x000021e42147c40d in AcpiInitializeObjects(UINT32) ../../third_party/acpica/source/components/utilities/utxfinit.c:304 <platform-bus-x86.so>+0x2fc40d
#13 0x000021e42126d603 in acpi::AcpiImpl::InitializeAcpi(acpi::AcpiImpl*) ../../src/devices/board/lib/acpi/acpi-impl.cc:224 <platform-bus-x86.so>+0xed603
Add a simple check that avoids incrementing a pointer by zero, but
otherwise behaves as before. Note that our findings are against ACPICA
20221020, but the same code exists on master.
| -rw-r--r-- | source/components/dispatcher/dswstate.c | 9 |
1 files changed, 7 insertions, 2 deletions
diff --git a/source/components/dispatcher/dswstate.c b/source/components/dispatcher/dswstate.c index f761ceb80..cc648e3ae 100644 --- a/source/components/dispatcher/dswstate.c +++ b/source/components/dispatcher/dswstate.c @@ -785,9 +785,14 @@ AcpiDsInitAmlWalk ( WalkState->ParserState.Aml = - WalkState->ParserState.AmlStart = AmlStart; + WalkState->ParserState.AmlStart = WalkState->ParserState.AmlEnd = - WalkState->ParserState.PkgEnd = AmlStart + AmlLength; + WalkState->ParserState.PkgEnd = AmlStart; + /* Avoid undefined behavior: applying zero offset to null pointer */ + if (AmlLength != 0) { + WalkState->ParserState.AmlEnd += AmlLength; + WalkState->ParserState.PkgEnd += AmlLength; + } /* The NextOp of the NextWalk will be the beginning of the method */ |