authorgravatar for mlugg@mlugg.co.ukMatthew Lugg <mlugg@mlugg.co.uk> 2025-08-27 21:35:45+01:00
committergravatar for mlugg@mlugg.co.ukMatthew Lugg <mlugg@mlugg.co.uk> 2025-08-28 10:56:11+01:00
loga31950aa578824e0933b49109f6ac55c84979b6d
treef704452b38410eb4ec1f1dc7139a6807c9ffc767
parent151c7dc74b97d2552637af526cf56390586386a6

std.debug: remove `@frameAddress()` "UAF"

We can't call `@frameAddress()` and then immediately `return`! That invalidates the frame. This *usually* isn't a problem, because the stack walk `next` call will *probably* have a stack frame and it will *probably* be at the exact same address, but neither of those is a guarantee. On powerpc, presumably some unfortunate inlining was going on, so this frame was indeed invalidated when we started walking frames. We need to explicitly pass `@frameAddress` into any function which will return before we actually walk the stack. Pretty simple patch. Resolves: #24970

2 files changed, 10 insertions(+), 20 deletions(-)

lib/std/debug.zig+9-19
...@@ -440,7 +440,7 @@ pub fn dumpStackTraceFromBase(context: *ThreadContext, stderr: *Writer) void {...@@ -440,7 +440,7 @@ pub fn dumpStackTraceFromBase(context: *ThreadContext, stderr: *Writer) void {
440 return;440 return;
441 }441 }
442442
443 var it = StackIterator.initWithContext(null, debug_info, context) catch return;443 var it = StackIterator.initWithContext(null, debug_info, context, @frameAddress()) catch return;
444 defer it.deinit();444 defer it.deinit();
445445
446 // DWARF unwinding on aarch64-macos is not complete so we need to get pc address from mcontext446 // DWARF unwinding on aarch64-macos is not complete so we need to get pc address from mcontext
...@@ -499,12 +499,7 @@ pub fn captureStackTrace(first_address: ?usize, stack_trace: *std.builtin.StackT...@@ -499,12 +499,7 @@ pub fn captureStackTrace(first_address: ?usize, stack_trace: *std.builtin.StackT
499 // TODO: This should use the DWARF unwinder if .eh_frame_hdr is available (so that full debug info parsing isn't required).499 // TODO: This should use the DWARF unwinder if .eh_frame_hdr is available (so that full debug info parsing isn't required).
500 // A new path for loading SelfInfo needs to be created which will only attempt to parse in-memory sections, because500 // A new path for loading SelfInfo needs to be created which will only attempt to parse in-memory sections, because
501 // stopping to load other debug info (ie. source line info) from disk here is not required for unwinding.501 // stopping to load other debug info (ie. source line info) from disk here is not required for unwinding.
502 if (builtin.cpu.arch == .powerpc64) {502 var it = StackIterator.init(first_address, @frameAddress());
503 // https://github.com/ziglang/zig/issues/24970
504 stack_trace.index = 0;
505 return;
506 }
507 var it = StackIterator.init(first_address, null);
508 defer it.deinit();503 defer it.deinit();
509 for (stack_trace.instruction_addresses, 0..) |*addr, i| {504 for (stack_trace.instruction_addresses, 0..) |*addr, i| {
510 addr.* = it.next() orelse {505 addr.* = it.next() orelse {
...@@ -787,7 +782,7 @@ pub const StackIterator = struct {...@@ -787,7 +782,7 @@ pub const StackIterator = struct {
787 failed: bool = false,782 failed: bool = false,
788 } else void = if (have_ucontext) null else {},783 } else void = if (have_ucontext) null else {},
789784
790 pub fn init(first_address: ?usize, fp: ?usize) StackIterator {785 pub fn init(first_address: ?usize, fp: usize) StackIterator {
791 if (native_arch.isSPARC()) {786 if (native_arch.isSPARC()) {
792 // Flush all the register windows on stack.787 // Flush all the register windows on stack.
793 asm volatile (if (builtin.cpu.has(.sparc, .v9))788 asm volatile (if (builtin.cpu.has(.sparc, .v9))
...@@ -799,23 +794,18 @@ pub const StackIterator = struct {...@@ -799,23 +794,18 @@ pub const StackIterator = struct {
799794
800 return .{795 return .{
801 .first_address = first_address,796 .first_address = first_address,
802 // TODO: this is a workaround for #16876797 .fp = fp,
803 //.fp = fp orelse @frameAddress(),
804 .fp = fp orelse blk: {
805 const fa = @frameAddress();
806 break :blk fa;
807 },
808 };798 };
809 }799 }
810800
811 pub fn initWithContext(first_address: ?usize, debug_info: *SelfInfo, context: *posix.ucontext_t) !StackIterator {801 pub fn initWithContext(first_address: ?usize, debug_info: *SelfInfo, context: *posix.ucontext_t, fp: usize) !StackIterator {
812 // The implementation of DWARF unwinding on aarch64-macos is not complete. However, Apple mandates that802 // The implementation of DWARF unwinding on aarch64-macos is not complete. However, Apple mandates that
813 // the frame pointer register is always used, so on this platform we can safely use the FP-based unwinder.803 // the frame pointer register is always used, so on this platform we can safely use the FP-based unwinder.
814 if (builtin.target.os.tag.isDarwin() and native_arch == .aarch64)804 if (builtin.target.os.tag.isDarwin() and native_arch == .aarch64)
815 return init(first_address, @truncate(context.mcontext.ss.fp));805 return init(first_address, @truncate(context.mcontext.ss.fp));
816806
817 if (SelfInfo.supports_unwinding) {807 if (SelfInfo.supports_unwinding) {
818 var iterator = init(first_address, null);808 var iterator = init(first_address, fp);
819 iterator.unwind_state = .{809 iterator.unwind_state = .{
820 .debug_info = debug_info,810 .debug_info = debug_info,
821 .dwarf_context = try SelfInfo.UnwindContext.init(debug_info.allocator, context),811 .dwarf_context = try SelfInfo.UnwindContext.init(debug_info.allocator, context),
...@@ -823,7 +813,7 @@ pub const StackIterator = struct {...@@ -823,7 +813,7 @@ pub const StackIterator = struct {
823 return iterator;813 return iterator;
824 }814 }
825815
826 return init(first_address, null);816 return init(first_address, fp);
827 }817 }
828818
829 pub fn deinit(it: *StackIterator) void {819 pub fn deinit(it: *StackIterator) void {
...@@ -981,8 +971,8 @@ pub fn writeCurrentStackTrace(...@@ -981,8 +971,8 @@ pub fn writeCurrentStackTrace(
981 const has_context = getContext(&context);971 const has_context = getContext(&context);
982972
983 var it = (if (has_context) blk: {973 var it = (if (has_context) blk: {
984 break :blk StackIterator.initWithContext(start_addr, debug_info, &context) catch null;974 break :blk StackIterator.initWithContext(start_addr, debug_info, &context, @frameAddress()) catch null;
985 } else null) orelse StackIterator.init(start_addr, null);975 } else null) orelse StackIterator.init(start_addr, @frameAddress());
986 defer it.deinit();976 defer it.deinit();
987977
988 while (it.next()) |return_address| {978 while (it.next()) |return_address| {
test/standalone/stack_iterator/unwind.zig+1-1
...@@ -10,7 +10,7 @@ noinline fn frame3(expected: *[4]usize, unwound: *[4]usize) void {...@@ -10,7 +10,7 @@ noinline fn frame3(expected: *[4]usize, unwound: *[4]usize) void {
10 testing.expect(debug.getContext(&context)) catch @panic("failed to getContext");10 testing.expect(debug.getContext(&context)) catch @panic("failed to getContext");
1111
12 const debug_info = debug.getSelfDebugInfo() catch @panic("failed to openSelfDebugInfo");12 const debug_info = debug.getSelfDebugInfo() catch @panic("failed to openSelfDebugInfo");
13 var it = debug.StackIterator.initWithContext(expected[0], debug_info, &context) catch @panic("failed to initWithContext");13 var it = debug.StackIterator.initWithContext(expected[0], debug_info, &context, @frameAddress()) catch @panic("failed to initWithContext");
14 defer it.deinit();14 defer it.deinit();
1515
16 for (unwound) |*addr| {16 for (unwound) |*addr| {