llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-libunwind Author: Thomas Anderson (tanderson-google) <details> <summary>Changes</summary> `evaluateExpression` interprets DW_OP_* bytecode against a fixed-size `pint_t stack[100]` operand stack without checking whether `sp` stays within array bounds. Malformed or hostile DWARF expressions (such as those injected via dynamic FDE registration) pushing more than 100 values or underflowing the stack could corrupt the native call stack. Add bounds checking on `sp` within `evaluateExpression`: - Bound `sp` to `[&stack[1], &stack[98]]` at each iteration in the opcode loop, as single-step opcodes push at most 1 value and net-pop at most 1. - Add index/depth validation for `DW_OP_pick` and `DW_OP_rot`. - Ensure `sp >= &stack[1]` on exit from the evaluator. - Abort execution via `_LIBUNWIND_ABORT` when bounds are violated. - Add a regression test verifying that an overflowing DWARF expression aborts safely rather than overflowing the stack. --- Full diff: https://github.com/llvm/llvm-project/pull/220662.diff 2 Files Affected: - (modified) libunwind/src/DwarfInstructions.hpp (+15) - (added) libunwind/test/dwarf_expression_stack.pass.cpp (+79) ``````````diff diff --git a/libunwind/src/DwarfInstructions.hpp b/libunwind/src/DwarfInstructions.hpp index 8868932d6821f..4f156586922f9 100644 --- a/libunwind/src/DwarfInstructions.hpp +++ b/libunwind/src/DwarfInstructions.hpp @@ -452,6 +452,15 @@ DwarfInstructions<A, R>::evaluateExpression(pint_t expression, A &addressSpace, *(++sp) = initialStackValue; while (p < expressionEnd) { + // Bounds-check the operand stack. Every opcode below pushes at most one + // value (writing at most sp[1]) and, except for DW_OP_pick and DW_OP_rot + // (checked at their use), reads/writes no deeper than sp[-1]. Keeping sp + // within [&stack[1], &stack[98]] here therefore bounds every access to + // the fixed-size array. Compiler-emitted CFI expressions use tiny stack + // depths; violating these bounds means corrupted or malicious unwind + // data (e.g. a hostile FDE registered via __register_frame()). + if (sp < &stack[1] || sp > &stack[98]) + _LIBUNWIND_ABORT("DWARF expression operand stack out of bounds"); if (log) { for (pint_t *t = sp; t > stack; --t) { fprintf(stderr, "sp[] = 0x%" PRIx64 "\n", (uint64_t)(*t)); @@ -594,6 +603,8 @@ DwarfInstructions<A, R>::evaluateExpression(pint_t expression, A &addressSpace, // pick from reg = addressSpace.get8(p); p += 1; + if (sp - (int)reg < &stack[1]) + _LIBUNWIND_ABORT("DW_OP_pick index out of bounds"); value = sp[-(int)reg]; *(++sp) = value; if (log) @@ -611,6 +622,8 @@ DwarfInstructions<A, R>::evaluateExpression(pint_t expression, A &addressSpace, case DW_OP_rot: // rotate top three + if (sp < &stack[3]) + _LIBUNWIND_ABORT("DW_OP_rot with fewer than three stack entries"); value = sp[0]; sp[0] = sp[-1]; sp[-1] = sp[-2]; @@ -970,6 +983,8 @@ DwarfInstructions<A, R>::evaluateExpression(pint_t expression, A &addressSpace, } } + if (sp < &stack[1]) + _LIBUNWIND_ABORT("DWARF expression operand stack out of bounds"); if (log) fprintf(stderr, "expression evaluates to 0x%" PRIx64 "\n", (uint64_t)*sp); return *sp; diff --git a/libunwind/test/dwarf_expression_stack.pass.cpp b/libunwind/test/dwarf_expression_stack.pass.cpp new file mode 100644 index 0000000000000..44e1757139dd4 --- /dev/null +++ b/libunwind/test/dwarf_expression_stack.pass.cpp @@ -0,0 +1,79 @@ +// -*- C++ -*- +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// Ensure that evaluateExpression bounds checks its internal operand stack and +// aborts on malformed DWARF expressions rather than overflowing the stack. +// REQUIRES: target={{(aarch64|x86_64)-.+}} +// UNSUPPORTED: target={{.*-windows.*}} +// UNSUPPORTED: target={{.*-apple.*}} + +// GCC doesn't support __attribute__((naked)) on AArch64. +// UNSUPPORTED: gcc + +// Inline assembly is incompatible with MSAN. +// UNSUPPORTED: msan + +#undef NDEBUG +#include <assert.h> +#include <libunwind.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <unistd.h> + +extern "C" void stepper() { + unw_cursor_t cursor; + unw_context_t uc; + unw_getcontext(&uc); + unw_init_local(&cursor, &uc); + // Stepping to bad_cfa_expression should evaluate the CFA expression + // and trigger _LIBUNWIND_ABORT due to stack overflow. + unw_step(&cursor); +} + +__attribute__((naked)) void bad_cfa_expression() { +#if defined(__aarch64__) + __asm__( + "stp x29, x30, [sp, #-16]!\n" + "mov x29, sp\n" + // DW_CFA_def_cfa_expression (0x0f), length 4, expression: DW_OP_dup (0x12), DW_OP_skip (0x2f) -4 (0xfc, 0xff) + ".cfi_escape 0x0f, 0x04, 0x12, 0x2f, 0xfc, 0xff\n" + "bl stepper\n" + "ldp x29, x30, [sp], #16\n" + "ret\n"); +#elif defined(__x86_64__) + __asm__( + "pushq %rbp\n" + "movq %rsp, %rbp\n" + // DW_CFA_def_cfa_expression (0x0f), length 4, expression: DW_OP_dup (0x12), DW_OP_skip (0x2f) -4 (0xfc, 0xff) + ".cfi_escape 0x0f, 0x04, 0x12, 0x2f, 0xfc, 0xff\n" + "callq stepper\n" + "popq %rbp\n" + "ret\n"); +#else +#error This test is only supported on aarch64 or x86-64 +#endif +} + +int main(int, char **) { + pid_t pid = fork(); + assert(pid >= 0); + if (pid == 0) { + bad_cfa_expression(); + exit(0); + } + + int status = 0; + waitpid(pid, &status, 0); + // The child process should abort due to operand stack bounds violation. + assert(WIFSIGNALED(status) && WTERMSIG(status) == SIGABRT); + return 0; +} `````````` </details> https://github.com/llvm/llvm-project/pull/220662 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
