From d1e8b73939ae9f139d90157a4623c5fe68b0ae64 Mon Sep 17 00:00:00 2001 From: Jan Philipp Hafer Date: Mon, 2 May 2022 00:27:02 +0200 Subject: [PATCH 1/2] std.os.abort: ported signal handling from musl * Document deviation from Linux man page, which is identical to musl. Man page wants always enabled user-provided abort handlers. Worst case logic bug, which this can introduce: + user disables SIGABRT handler to prevent tear down to last safe state + abort() gets called and enables user-provided SIGABRT handler + SIGABRT tears down to supposed last safe state instead of crash + Application, instead of crashing, continues * Pid 1 within containers needs special handling. - fatal signals are not transmitted without privileges, so use exit as fallback * Fix some signaling bits * Add checks in Debug and ReleaseSafe for wrong sigprocmask --- lib/std/os.zig | 64 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index 18513f2eca..d32618a82f 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -250,6 +250,15 @@ pub var argv: [][*:0]u8 = if (builtin.link_libc) undefined else switch (builtin. else => undefined, }; +/// Atomic to guard correct program teardown in abort() +var abort_entered = impl: { + if (builtin.single_threaded) { + break :impl {}; + } else { + break :impl std.atomic.Atomic(bool).init(false); + } +}; + /// To obtain errno, call this function with the return value of the /// system function call. For some systems this will obtain the value directly /// from the return code; for others it will use a thread-local errno variable. @@ -442,6 +451,7 @@ fn getRandomBytesDevURandom(buf: []u8) !void { /// Causes abnormal process termination. /// If linking against libc, this calls the abort() libc function. Otherwise /// it raises SIGABRT followed by SIGKILL and finally lo +/// assume: Current signal handler for SIGABRT does **not call abort**. pub fn abort() noreturn { @setCold(true); // MSVCRT abort() sometimes opens a popup window which is undesirable, so @@ -454,11 +464,48 @@ pub fn abort() noreturn { windows.kernel32.ExitProcess(3); } if (!builtin.link_libc and builtin.os.tag == .linux) { + // Linux man page wants to first "unblock SIG.ABRT", but this is a footgun + // for user-defined signal handlers that want to restore some state in + // some program sections and crash in others + + // user installed SIGABRT handler is run, if installed raise(SIG.ABRT) catch {}; - // TODO the rest of the implementation of abort() from musl libc here + // disable all signal handlers + sigprocmask(SIG.BLOCK, &linux.all_mask, null); + // ensure teardown by one thread + if (!builtin.single_threaded) { + while (abort_entered.compareAndSwap(false, true, .SeqCst, .SeqCst)) |_| {} + } + + // install default handler to terminate + const sigact = Sigaction{ + .handler = .{ .sigaction = SIG.DFL }, + .mask = undefined, + .flags = undefined, + .restorer = undefined, + }; + sigaction(SIG.ABRT, &sigact, null) catch unreachable; + + // make sure we have a pending SIGABRT queued + const tid = std.Thread.getCurrentId(); + _ = linux.tkill(@intCast(i32, tid), SIG.ABRT); + + // SIG.ABRT signal will run default handler + const sigabrtmask: linux.sigset_t = [_]u32{0} ** 31 ++ [_]u32{1 << (SIG.ABRT - 1)}; + sigprocmask(SIG.UNBLOCK, &sigabrtmask, null); // [32]u32 + + // Beyond this point should be unreachable + + // abnormal termination without using signal handler + const nullptr: *allowzero volatile u8 = @intToPtr(*allowzero volatile u8, 0); + nullptr.* = 0; + + // try SIGKILL, which is no abnormal termination as defined by POSIX and ISO C raise(SIG.KILL) catch {}; + + // pid 1 might not be signalled in some containers exit(127); } if (builtin.os.tag == .uefi) { @@ -485,13 +532,13 @@ pub fn raise(sig: u8) RaiseError!void { if (builtin.os.tag == .linux) { var set: sigset_t = undefined; // block application signals - _ = linux.sigprocmask(SIG.BLOCK, &linux.app_mask, &set); + sigprocmask(SIG.BLOCK, &linux.app_mask, &set); const tid = linux.gettid(); const rc = linux.tkill(tid, sig); // restore signal mask - _ = linux.sigprocmask(SIG.SETMASK, &set, null); + sigprocmask(SIG.SETMASK, &set, null); switch (errno(rc)) { .SUCCESS => return, @@ -5441,6 +5488,17 @@ pub fn sigaction(sig: u6, noalias act: ?*const Sigaction, noalias oact: ?*Sigact } } +/// Set the thread signal mask +/// Invalid masks are checked in Debug and ReleaseFast +pub fn sigprocmask(flags: u32, noalias set: ?*const sigset_t, noalias oldset: ?*sigset_t) void { + switch (errno(system.sigprocmask(flags, set, oldset))) { + .SUCCESS => return, + .FAULT => unreachable, + .INVAL => unreachable, // main purpose: debug InvalidValue error + else => unreachable, + } +} + pub const FutimensError = error{ /// times is NULL, or both tv_nsec values are UTIME_NOW, and either: /// * the effective user ID of the caller does not match the owner From 073762395ef3beddf458d87a6b7aad4342b9af8e Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 6 Jun 2022 15:29:34 -0700 Subject: [PATCH 2/2] std.os.abort patch cleanups * move global into function scope * clarify comments * avoid unnecessary usage of std.atomic API * switch on error instead of `catch unreachable` * call linux.gettid() instead of going through higher level API and doing unnecessary casting --- lib/std/os.zig | 59 +++++++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index d32618a82f..c79bd6e7b0 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -250,15 +250,6 @@ pub var argv: [][*:0]u8 = if (builtin.link_libc) undefined else switch (builtin. else => undefined, }; -/// Atomic to guard correct program teardown in abort() -var abort_entered = impl: { - if (builtin.single_threaded) { - break :impl {}; - } else { - break :impl std.atomic.Atomic(bool).init(false); - } -}; - /// To obtain errno, call this function with the return value of the /// system function call. For some systems this will obtain the value directly /// from the return code; for others it will use a thread-local errno variable. @@ -451,7 +442,7 @@ fn getRandomBytesDevURandom(buf: []u8) !void { /// Causes abnormal process termination. /// If linking against libc, this calls the abort() libc function. Otherwise /// it raises SIGABRT followed by SIGKILL and finally lo -/// assume: Current signal handler for SIGABRT does **not call abort**. +/// Invokes the current signal handler for SIGABRT, if any. pub fn abort() noreturn { @setCold(true); // MSVCRT abort() sometimes opens a popup window which is undesirable, so @@ -464,49 +455,44 @@ pub fn abort() noreturn { windows.kernel32.ExitProcess(3); } if (!builtin.link_libc and builtin.os.tag == .linux) { - // Linux man page wants to first "unblock SIG.ABRT", but this is a footgun + // The Linux man page says that the libc abort() function + // "first unblocks the SIGABRT signal", but this is a footgun // for user-defined signal handlers that want to restore some state in - // some program sections and crash in others - - // user installed SIGABRT handler is run, if installed + // some program sections and crash in others. + // So, the user-installed SIGABRT handler is run, if present. raise(SIG.ABRT) catch {}; - // disable all signal handlers + // Disable all signal handlers. sigprocmask(SIG.BLOCK, &linux.all_mask, null); - // ensure teardown by one thread + // Only one thread may proceed to the rest of abort(). if (!builtin.single_threaded) { - while (abort_entered.compareAndSwap(false, true, .SeqCst, .SeqCst)) |_| {} + const global = struct { + var abort_entered: bool = false; + }; + while (@cmpxchgWeak(bool, &global.abort_entered, false, true, .SeqCst, .SeqCst)) |_| {} } - // install default handler to terminate + // Install default handler so that the tkill below will terminate. const sigact = Sigaction{ .handler = .{ .sigaction = SIG.DFL }, .mask = undefined, .flags = undefined, .restorer = undefined, }; - sigaction(SIG.ABRT, &sigact, null) catch unreachable; + sigaction(SIG.ABRT, &sigact, null) catch |err| switch (err) { + error.OperationNotSupported => unreachable, + }; - // make sure we have a pending SIGABRT queued - const tid = std.Thread.getCurrentId(); - _ = linux.tkill(@intCast(i32, tid), SIG.ABRT); + _ = linux.tkill(linux.gettid(), SIG.ABRT); - // SIG.ABRT signal will run default handler const sigabrtmask: linux.sigset_t = [_]u32{0} ** 31 ++ [_]u32{1 << (SIG.ABRT - 1)}; - sigprocmask(SIG.UNBLOCK, &sigabrtmask, null); // [32]u32 + sigprocmask(SIG.UNBLOCK, &sigabrtmask, null); - // Beyond this point should be unreachable - - // abnormal termination without using signal handler - const nullptr: *allowzero volatile u8 = @intToPtr(*allowzero volatile u8, 0); - nullptr.* = 0; - - // try SIGKILL, which is no abnormal termination as defined by POSIX and ISO C + // Beyond this point should be unreachable. + @intToPtr(*allowzero volatile u8, 0).* = 0; raise(SIG.KILL) catch {}; - - // pid 1 might not be signalled in some containers - exit(127); + exit(127); // Pid 1 might not be signalled in some containers. } if (builtin.os.tag == .uefi) { exit(0); // TODO choose appropriate exit code @@ -5488,13 +5474,12 @@ pub fn sigaction(sig: u6, noalias act: ?*const Sigaction, noalias oact: ?*Sigact } } -/// Set the thread signal mask -/// Invalid masks are checked in Debug and ReleaseFast +/// Sets the thread signal mask. pub fn sigprocmask(flags: u32, noalias set: ?*const sigset_t, noalias oldset: ?*sigset_t) void { switch (errno(system.sigprocmask(flags, set, oldset))) { .SUCCESS => return, .FAULT => unreachable, - .INVAL => unreachable, // main purpose: debug InvalidValue error + .INVAL => unreachable, else => unreachable, } }