From c0b7f20893ea5ca42e0d02b59db6f459c2f80ca1 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 17 Aug 2022 18:42:39 -0700 Subject: [PATCH] stage2: implement stack protectors This is one of the final remaining TODOs for the LLVM backend. --- src/Compilation.zig | 121 +++++++++++++++++++++------------ src/clang_options_data.zig | 45 ++++++++++-- src/codegen/llvm.zig | 11 ++- src/glibc.zig | 1 + src/libcxx.zig | 2 + src/libtsan.zig | 1 + src/libunwind.zig | 1 + src/link.zig | 3 + src/link/Elf.zig | 6 ++ src/main.zig | 16 +++++ src/musl.zig | 1 + src/target.zig | 9 +++ tools/update_clang_options.zig | 20 ++++++ 13 files changed, 184 insertions(+), 53 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index 5e0d815c96..060afba694 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -173,6 +173,7 @@ astgen_wait_group: WaitGroup = .{}, /// TODO: Remove this when Stage2 becomes the default compiler as it will already have this information. export_symbol_names: std.ArrayListUnmanaged([]const u8) = .{}, +pub const default_stack_protector_buffer_size = 4; pub const SemaError = Module.SemaError; pub const CRTFile = struct { @@ -837,6 +838,10 @@ pub const InitOptions = struct { want_pie: ?bool = null, want_sanitize_c: ?bool = null, want_stack_check: ?bool = null, + /// null means default. + /// 0 means no stack protector. + /// other number means stack protection with that buffer size. + want_stack_protector: ?u32 = null, want_red_zone: ?bool = null, omit_frame_pointer: ?bool = null, want_valgrind: ?bool = null, @@ -1014,6 +1019,15 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { return error.ExportTableAndImportTableConflict; } + // The `have_llvm` condition is here only because native backends cannot yet build compiler-rt. + // Once they are capable this condition could be removed. When removing this condition, + // also test the use case of `build-obj -fcompiler-rt` with the native backends + // and make sure the compiler-rt symbols are emitted. + const capable_of_building_compiler_rt = build_options.have_llvm; + + const capable_of_building_zig_libc = build_options.have_llvm; + const capable_of_building_ssp = build_options.have_llvm; + const comp: *Compilation = comp: { // For allocations that have the same lifetime as Compilation. This arena is used only during this // initialization and then is freed in deinit(). @@ -1289,11 +1303,36 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { const sanitize_c = options.want_sanitize_c orelse is_safe_mode; - const stack_check: bool = b: { - if (!target_util.supportsStackProbing(options.target)) - break :b false; - break :b options.want_stack_check orelse is_safe_mode; + const stack_check: bool = options.want_stack_check orelse b: { + if (!target_util.supportsStackProbing(options.target)) break :b false; + break :b is_safe_mode; }; + if (stack_check and !target_util.supportsStackProbing(options.target)) + return error.StackCheckUnsupportedByTarget; + + const stack_protector: u32 = options.want_stack_protector orelse b: { + if (!target_util.supportsStackProtector(options.target)) break :b @as(u32, 0); + + // This logic is checking for linking libc because otherwise our start code + // which is trying to set up TLS (i.e. the fs/gs registers) but the stack + // protection code depends on fs/gs registers being already set up. + // If we were able to annotate start code, or perhaps the entire std lib, + // as being exempt from stack protection checks, we could change this logic + // to supporting stack protection even when not linking libc. + // TODO file issue about this + if (!link_libc) break :b 0; + if (!capable_of_building_ssp) break :b 0; + if (is_safe_mode) break :b default_stack_protector_buffer_size; + break :b 0; + }; + if (stack_protector != 0) { + if (!target_util.supportsStackProtector(options.target)) + return error.StackProtectorUnsupportedByTarget; + if (!capable_of_building_ssp) + return error.StackProtectorUnsupportedByBackend; + if (!link_libc) + return error.StackProtectorUnavailableWithoutLibC; + } const valgrind: bool = b: { if (!target_util.hasValgrindSupport(options.target)) @@ -1378,6 +1417,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { cache.hash.add(unwind_tables); cache.hash.add(tsan); cache.hash.add(stack_check); + cache.hash.add(stack_protector); cache.hash.add(red_zone); cache.hash.add(omit_frame_pointer); cache.hash.add(link_mode); @@ -1741,6 +1781,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { .valgrind = valgrind, .tsan = tsan, .stack_check = stack_check, + .stack_protector = stack_protector, .red_zone = red_zone, .omit_frame_pointer = omit_frame_pointer, .single_threaded = single_threaded, @@ -1822,6 +1863,8 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { }; errdefer comp.destroy(); + const target = comp.getTarget(); + // Add a `CObject` for each `c_source_files`. try comp.c_object_table.ensureTotalCapacity(gpa, options.c_source_files.len); for (options.c_source_files) |c_source_file| { @@ -1837,11 +1880,9 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { const have_bin_emit = comp.bin_file.options.emit != null or comp.whole_bin_sub_path != null; - if (have_bin_emit and !comp.bin_file.options.skip_linker_dependencies and - options.target.ofmt != .c) - { - if (comp.getTarget().isDarwin()) { - switch (comp.getTarget().abi) { + if (have_bin_emit and !comp.bin_file.options.skip_linker_dependencies and target.ofmt != .c) { + if (target.isDarwin()) { + switch (target.abi) { .none, .simulator, .macabi, @@ -1852,9 +1893,9 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { // If we need to build glibc for the target, add work items for it. // We go through the work queue so that building can be done in parallel. if (comp.wantBuildGLibCFromSource()) { - if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable; + if (!target_util.canBuildLibC(target)) return error.LibCUnavailable; - if (glibc.needsCrtiCrtn(comp.getTarget())) { + if (glibc.needsCrtiCrtn(target)) { try comp.work_queue.write(&[_]Job{ .{ .glibc_crt_file = .crti_o }, .{ .glibc_crt_file = .crtn_o }, @@ -1867,10 +1908,10 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { }); } if (comp.wantBuildMuslFromSource()) { - if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable; + if (!target_util.canBuildLibC(target)) return error.LibCUnavailable; try comp.work_queue.ensureUnusedCapacity(6); - if (musl.needsCrtiCrtn(comp.getTarget())) { + if (musl.needsCrtiCrtn(target)) { comp.work_queue.writeAssumeCapacity(&[_]Job{ .{ .musl_crt_file = .crti_o }, .{ .musl_crt_file = .crtn_o }, @@ -1887,7 +1928,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { }); } if (comp.wantBuildWasiLibcFromSource()) { - if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable; + if (!target_util.canBuildLibC(target)) return error.LibCUnavailable; const wasi_emulated_libs = comp.bin_file.options.wasi_emulated_libs; try comp.work_queue.ensureUnusedCapacity(wasi_emulated_libs.len + 2); // worst-case we need all components @@ -1902,7 +1943,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { }); } if (comp.wantBuildMinGWFromSource()) { - if (!target_util.canBuildLibC(comp.getTarget())) return error.LibCUnavailable; + if (!target_util.canBuildLibC(target)) return error.LibCUnavailable; const static_lib_jobs = [_]Job{ .{ .mingw_crt_file = .mingw32_lib }, @@ -1921,7 +1962,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { } } // Generate Windows import libs. - if (comp.getTarget().os.tag == .windows) { + if (target.os.tag == .windows) { const count = comp.bin_file.options.system_libs.count(); try comp.work_queue.ensureUnusedCapacity(count); var i: usize = 0; @@ -1940,15 +1981,6 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { try comp.work_queue.writeItem(.libtsan); } - // The `have_llvm` condition is here only because native backends cannot yet build compiler-rt. - // Once they are capable this condition could be removed. When removing this condition, - // also test the use case of `build-obj -fcompiler-rt` with the native backends - // and make sure the compiler-rt symbols are emitted. - const capable_of_building_compiler_rt = build_options.have_llvm; - - const capable_of_building_zig_libc = build_options.have_llvm; - const capable_of_building_ssp = comp.bin_file.options.use_stage1; - if (comp.bin_file.options.include_compiler_rt and capable_of_building_compiler_rt) { if (is_exe_or_dyn_lib) { log.debug("queuing a job to build compiler_rt_lib", .{}); @@ -1962,8 +1994,11 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { } } if (needs_c_symbols) { - // MinGW provides no libssp, use our own implementation. - if (comp.getTarget().isMinGW() and capable_of_building_ssp) { + // Related: https://github.com/ziglang/zig/issues/7265. + if (comp.bin_file.options.stack_protector != 0 and + (!comp.bin_file.options.link_libc or + !target_util.libcProvidesStackProtector(target))) + { try comp.work_queue.writeItem(.{ .libssp = {} }); } @@ -4123,6 +4158,17 @@ pub fn addCCArgs( try argv.append("-fno-omit-frame-pointer"); } + const ssp_buf_size = comp.bin_file.options.stack_protector; + if (ssp_buf_size != 0) { + try argv.appendSlice(&[_][]const u8{ + "-fstack-protector-strong", + "--param", + try std.fmt.allocPrint(arena, "ssp-buffer-size={d}", .{ssp_buf_size}), + }); + } else { + try argv.append("-fno-stack-protector"); + } + switch (comp.bin_file.options.optimize_mode) { .Debug => { // windows c runtime requires -D_DEBUG if using debug libraries @@ -4131,27 +4177,12 @@ pub fn addCCArgs( // to -O1. Besides potentially impairing debugging, -O1/-Og significantly // increases compile times. try argv.append("-O0"); - - if (comp.bin_file.options.link_libc and target.os.tag != .wasi) { - try argv.append("-fstack-protector-strong"); - try argv.append("--param"); - try argv.append("ssp-buffer-size=4"); - } else { - try argv.append("-fno-stack-protector"); - } }, .ReleaseSafe => { // See the comment in the BuildModeFastRelease case for why we pass -O2 rather // than -O3 here. try argv.append("-O2"); - if (comp.bin_file.options.link_libc and target.os.tag != .wasi) { - try argv.append("-D_FORTIFY_SOURCE=2"); - try argv.append("-fstack-protector-strong"); - try argv.append("--param"); - try argv.append("ssp-buffer-size=4"); - } else { - try argv.append("-fno-stack-protector"); - } + try argv.append("-D_FORTIFY_SOURCE=2"); }, .ReleaseFast => { try argv.append("-DNDEBUG"); @@ -4161,12 +4192,10 @@ pub fn addCCArgs( // Zig code than it is for C code. Also, C programmers are used to their code // running in -O2 and thus the -O3 path has been tested less. try argv.append("-O2"); - try argv.append("-fno-stack-protector"); }, .ReleaseSmall => { try argv.append("-DNDEBUG"); try argv.append("-Os"); - try argv.append("-fno-stack-protector"); }, } @@ -5031,6 +5060,7 @@ fn buildOutputFromZig( .use_stage1 = build_options.is_stage1 and comp.bin_file.options.use_stage1, .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_red_zone = comp.bin_file.options.red_zone, .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer, .want_valgrind = false, @@ -5311,6 +5341,7 @@ pub fn build_crt_file( .optimize_mode = comp.compilerRtOptMode(), .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_red_zone = comp.bin_file.options.red_zone, .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer, .want_valgrind = false, diff --git a/src/clang_options_data.zig b/src/clang_options_data.zig index 76e687c7d6..90ba377847 100644 --- a/src/clang_options_data.zig +++ b/src/clang_options_data.zig @@ -3290,7 +3290,14 @@ flagpd1("fno-stack-arrays"), .psl = false, }, flagpd1("fno-stack-clash-protection"), -flagpd1("fno-stack-protector"), +.{ + .name = "fno-stack-protector", + .syntax = .flag, + .zig_equivalent = .no_stack_protector, + .pd1 = true, + .pd2 = false, + .psl = false, +}, flagpd1("fno-stack-size-section"), flagpd1("fno-standalone-debug"), flagpd1("fno-strength-reduce"), @@ -3588,9 +3595,30 @@ flagpd1("fstack-arrays"), .psl = false, }, flagpd1("fstack-clash-protection"), -flagpd1("fstack-protector"), -flagpd1("fstack-protector-all"), -flagpd1("fstack-protector-strong"), +.{ + .name = "fstack-protector", + .syntax = .flag, + .zig_equivalent = .stack_protector, + .pd1 = true, + .pd2 = false, + .psl = false, +}, +.{ + .name = "fstack-protector-all", + .syntax = .flag, + .zig_equivalent = .stack_protector, + .pd1 = true, + .pd2 = false, + .psl = false, +}, +.{ + .name = "fstack-protector-strong", + .syntax = .flag, + .zig_equivalent = .stack_protector, + .pd1 = true, + .pd2 = false, + .psl = false, +}, flagpd1("fstack-size-section"), flagpd1("fstack-usage"), flagpd1("fstandalone-debug"), @@ -4809,7 +4837,14 @@ flagpd1("single_module"), }, sepd1("split-dwarf-file"), sepd1("split-dwarf-output"), -sepd1("stack-protector"), +.{ + .name = "stack-protector", + .syntax = .separate, + .zig_equivalent = .stack_protector, + .pd1 = true, + .pd2 = false, + .psl = false, +}, sepd1("stack-protector-buffer-size"), sepd1("stack-usage-file"), .{ diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 8c84f61a81..0898c8fe87 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -711,9 +711,14 @@ pub const Object = struct { DeclGen.removeFnAttr(llvm_func, "noinline"); } - // TODO: port these over from stage1 - // addLLVMFnAttr(llvm_fn, "sspstrong"); - // addLLVMFnAttrStr(llvm_fn, "stack-protector-buffer-size", "4"); + // TODO: disable this if safety is off for the function scope + const ssp_buf_size = module.comp.bin_file.options.stack_protector; + if (ssp_buf_size != 0) { + var buf: [12]u8 = undefined; + const arg = std.fmt.bufPrintZ(&buf, "{d}", .{ssp_buf_size}) catch unreachable; + dg.addFnAttr(llvm_func, "sspstrong"); + dg.addFnAttrString(llvm_func, "stack-protector-buffer-size", arg); + } // TODO: disable this if safety is off for the function scope if (module.comp.bin_file.options.stack_check) { diff --git a/src/glibc.zig b/src/glibc.zig index 4deac5275f..ceb60ff096 100644 --- a/src/glibc.zig +++ b/src/glibc.zig @@ -1111,6 +1111,7 @@ fn buildSharedLib( .optimize_mode = comp.compilerRtOptMode(), .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_red_zone = comp.bin_file.options.red_zone, .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer, .want_valgrind = false, diff --git a/src/libcxx.zig b/src/libcxx.zig index 17d88fa03d..b19834f8c3 100644 --- a/src/libcxx.zig +++ b/src/libcxx.zig @@ -206,6 +206,7 @@ pub fn buildLibCXX(comp: *Compilation) !void { .link_mode = link_mode, .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_red_zone = comp.bin_file.options.red_zone, .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer, .want_valgrind = false, @@ -349,6 +350,7 @@ pub fn buildLibCXXABI(comp: *Compilation) !void { .link_mode = link_mode, .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_red_zone = comp.bin_file.options.red_zone, .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer, .want_valgrind = false, diff --git a/src/libtsan.zig b/src/libtsan.zig index cbbcd11a8a..16e40c16f8 100644 --- a/src/libtsan.zig +++ b/src/libtsan.zig @@ -211,6 +211,7 @@ pub fn buildTsan(comp: *Compilation) !void { .link_mode = link_mode, .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_valgrind = false, .want_tsan = false, .want_pic = true, diff --git a/src/libunwind.zig b/src/libunwind.zig index 0d030be329..fb4f93af02 100644 --- a/src/libunwind.zig +++ b/src/libunwind.zig @@ -113,6 +113,7 @@ pub fn buildStaticLib(comp: *Compilation) !void { .link_mode = link_mode, .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_red_zone = comp.bin_file.options.red_zone, .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer, .want_valgrind = false, diff --git a/src/link.zig b/src/link.zig index 31b54f705a..a0c0c5c369 100644 --- a/src/link.zig +++ b/src/link.zig @@ -90,6 +90,9 @@ pub const Options = struct { entry: ?[]const u8, stack_size_override: ?u64, image_base_override: ?u64, + /// 0 means no stack protector + /// other value means stack protector with that buffer size. + stack_protector: u32, cache_mode: CacheMode, include_compiler_rt: bool, /// Set to `true` to omit debug info. diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 2f67c35205..9902886bac 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -1673,6 +1673,12 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v } } + // stack-protector. + // Related: https://github.com/ziglang/zig/issues/7265 + if (comp.libssp_static_lib) |ssp| { + try argv.append(ssp.full_object_path); + } + // compiler-rt if (compiler_rt_path) |p| { try argv.append(p); diff --git a/src/main.zig b/src/main.zig index dfce4d83b8..91d171d237 100644 --- a/src/main.zig +++ b/src/main.zig @@ -378,6 +378,8 @@ const usage_build_generic = \\ -fno-lto Force-disable Link Time Optimization \\ -fstack-check Enable stack probing in unsafe builds \\ -fno-stack-check Disable stack probing in safe builds + \\ -fstack-protector Enable stack protection in unsafe builds + \\ -fno-stack-protector Disable stack protection in safe builds \\ -fsanitize-c Enable C undefined behavior detection in unsafe builds \\ -fno-sanitize-c Disable C undefined behavior detection in safe builds \\ -fvalgrind Include valgrind client requests in release builds @@ -668,6 +670,7 @@ fn buildOutputType( var want_unwind_tables: ?bool = null; var want_sanitize_c: ?bool = null; var want_stack_check: ?bool = null; + var want_stack_protector: ?u32 = null; var want_red_zone: ?bool = null; var omit_frame_pointer: ?bool = null; var want_valgrind: ?bool = null; @@ -1168,6 +1171,10 @@ fn buildOutputType( want_stack_check = true; } else if (mem.eql(u8, arg, "-fno-stack-check")) { want_stack_check = false; + } else if (mem.eql(u8, arg, "-fstack-protector")) { + want_stack_protector = Compilation.default_stack_protector_buffer_size; + } else if (mem.eql(u8, arg, "-fno-stack-protector")) { + want_stack_protector = 0; } else if (mem.eql(u8, arg, "-mred-zone")) { want_red_zone = true; } else if (mem.eql(u8, arg, "-mno-red-zone")) { @@ -1521,6 +1528,12 @@ fn buildOutputType( .no_color_diagnostics => color = .off, .stack_check => want_stack_check = true, .no_stack_check => want_stack_check = false, + .stack_protector => { + if (want_stack_protector == null) { + want_stack_protector = Compilation.default_stack_protector_buffer_size; + } + }, + .no_stack_protector => want_stack_protector = 0, .unwind_tables => want_unwind_tables = true, .no_unwind_tables => want_unwind_tables = false, .nostdlib => ensure_libc_on_non_freestanding = false, @@ -2859,6 +2872,7 @@ fn buildOutputType( .want_unwind_tables = want_unwind_tables, .want_sanitize_c = want_sanitize_c, .want_stack_check = want_stack_check, + .want_stack_protector = want_stack_protector, .want_red_zone = want_red_zone, .omit_frame_pointer = omit_frame_pointer, .want_valgrind = want_valgrind, @@ -4663,6 +4677,8 @@ pub const ClangArgIterator = struct { no_color_diagnostics, stack_check, no_stack_check, + stack_protector, + no_stack_protector, strip, exec_model, emit_llvm, diff --git a/src/musl.zig b/src/musl.zig index 68b524b415..12ff530f8e 100644 --- a/src/musl.zig +++ b/src/musl.zig @@ -215,6 +215,7 @@ pub fn buildCRTFile(comp: *Compilation, crt_file: CRTFile) !void { .optimize_mode = comp.compilerRtOptMode(), .want_sanitize_c = false, .want_stack_check = false, + .want_stack_protector = 0, .want_red_zone = comp.bin_file.options.red_zone, .omit_frame_pointer = comp.bin_file.options.omit_frame_pointer, .want_valgrind = false, diff --git a/src/target.zig b/src/target.zig index 5202fb15fc..3c8eabe01d 100644 --- a/src/target.zig +++ b/src/target.zig @@ -300,6 +300,15 @@ pub fn supportsStackProbing(target: std.Target) bool { (target.cpu.arch == .i386 or target.cpu.arch == .x86_64); } +pub fn supportsStackProtector(target: std.Target) bool { + _ = target; + return true; +} + +pub fn libcProvidesStackProtector(target: std.Target) bool { + return !target.isMinGW() and target.os.tag != .wasi; +} + pub fn supportsReturnAddress(target: std.Target) bool { return switch (target.cpu.arch) { .wasm32, .wasm64 => target.os.tag == .emscripten, diff --git a/tools/update_clang_options.zig b/tools/update_clang_options.zig index d261b5ee33..f1656546f0 100644 --- a/tools/update_clang_options.zig +++ b/tools/update_clang_options.zig @@ -352,6 +352,26 @@ const known_options = [_]KnownOpt{ .name = "fno-stack-check", .ident = "no_stack_check", }, + .{ + .name = "stack-protector", + .ident = "stack_protector", + }, + .{ + .name = "fstack-protector", + .ident = "stack_protector", + }, + .{ + .name = "fno-stack-protector", + .ident = "no_stack_protector", + }, + .{ + .name = "fstack-protector-strong", + .ident = "stack_protector", + }, + .{ + .name = "fstack-protector-all", + .ident = "stack_protector", + }, .{ .name = "MD", .ident = "dep_file",