Skip to content

Commit b071f85

Browse files
committed
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
1 parent 50edad3 commit b071f85

File tree

1 file changed

+9
-19
lines changed

1 file changed

+9
-19
lines changed

lib/std/debug.zig

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ pub fn dumpStackTraceFromBase(context: *ThreadContext, stderr: *Writer) void {
440440
return;
441441
}
442442

443-
var it = StackIterator.initWithContext(null, debug_info, context) catch return;
443+
var it = StackIterator.initWithContext(null, debug_info, context, @frameAddress()) catch return;
444444
defer it.deinit();
445445

446446
// 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
499499
// TODO: This should use the DWARF unwinder if .eh_frame_hdr is available (so that full debug info parsing isn't required).
500500
// A new path for loading SelfInfo needs to be created which will only attempt to parse in-memory sections, because
501501
// stopping to load other debug info (ie. source line info) from disk here is not required for unwinding.
502-
if (builtin.cpu.arch == .powerpc64) {
503-
// https://github.com/ziglang/zig/issues/24970
504-
stack_trace.index = 0;
505-
return;
506-
}
507-
var it = StackIterator.init(first_address, null);
502+
var it = StackIterator.init(first_address, @frameAddress());
508503
defer it.deinit();
509504
for (stack_trace.instruction_addresses, 0..) |*addr, i| {
510505
addr.* = it.next() orelse {
@@ -787,7 +782,7 @@ pub const StackIterator = struct {
787782
failed: bool = false,
788783
} 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 {
791786
if (native_arch.isSPARC()) {
792787
// Flush all the register windows on stack.
793788
asm volatile (if (builtin.cpu.has(.sparc, .v9))
@@ -799,31 +794,26 @@ pub const StackIterator = struct {
799794

800795
return .{
801796
.first_address = first_address,
802-
// TODO: this is a workaround for #16876
803-
//.fp = fp orelse @frameAddress(),
804-
.fp = fp orelse blk: {
805-
const fa = @frameAddress();
806-
break :blk fa;
807-
},
797+
.fp = fp,
808798
};
809799
}
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 {
812802
// The implementation of DWARF unwinding on aarch64-macos is not complete. However, Apple mandates that
813803
// the frame pointer register is always used, so on this platform we can safely use the FP-based unwinder.
814804
if (builtin.target.os.tag.isDarwin() and native_arch == .aarch64)
815805
return init(first_address, @truncate(context.mcontext.ss.fp));
816806

817807
if (SelfInfo.supports_unwinding) {
818-
var iterator = init(first_address, null);
808+
var iterator = init(first_address, fp);
819809
iterator.unwind_state = .{
820810
.debug_info = debug_info,
821811
.dwarf_context = try SelfInfo.UnwindContext.init(debug_info.allocator, context),
822812
};
823813
return iterator;
824814
}
825815

826-
return init(first_address, null);
816+
return init(first_address, fp);
827817
}
828818

829819
pub fn deinit(it: *StackIterator) void {
@@ -981,8 +971,8 @@ pub fn writeCurrentStackTrace(
981971
const has_context = getContext(&context);
982972

983973
var it = (if (has_context) blk: {
984-
break :blk StackIterator.initWithContext(start_addr, debug_info, &context) catch null;
985-
} else null) orelse StackIterator.init(start_addr, null);
974+
break :blk StackIterator.initWithContext(start_addr, debug_info, &context, @frameAddress()) catch null;
975+
} else null) orelse StackIterator.init(start_addr, @frameAddress());
986976
defer it.deinit();
987977

988978
while (it.next()) |return_address| {

0 commit comments

Comments
 (0)