From 992c02ab95e8297a1558bcf011f15a5cf1cd8e1b Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Thu, 20 May 2021 14:43:04 +0200 Subject: [PATCH 1/3] std: Fix error in tlcsprng init sequence The fallback case was actually switched with the success one. --- lib/std/crypto/tlcsprng.zig | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/std/crypto/tlcsprng.zig b/lib/std/crypto/tlcsprng.zig index 8a2c24916b..1e39b125f6 100644 --- a/lib/std/crypto/tlcsprng.zig +++ b/lib/std/crypto/tlcsprng.zig @@ -107,13 +107,9 @@ fn tlsCsprngFill(_: *const std.rand.Random, buffer: []u8) void { break :wof; } else |_| {} - os.madvise( - wipe_mem.ptr, - wipe_mem.len, - os.MADV_WIPEONFORK, - ) catch { + if (os.madvise(wipe_mem.ptr, wipe_mem.len, os.MADV_WIPEONFORK)) |_| { return initAndFill(buffer); - }; + } else |_| {} } if (std.Thread.use_pthreads) { From abfe7f96dd45e22d43c3e14ed64e1fbb8e94b419 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Thu, 20 May 2021 15:26:17 +0200 Subject: [PATCH 2/3] std: Call pthread_atfork only once Some libc implementations (glib) deduplicate identical hooks, others (musl, macos) do not and blindly append them to an internal list. Ensure there's only a single call to pthread_atfork to prevent unbounded memory use when lots of threads/forks are used. --- lib/std/crypto/tlcsprng.zig | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/std/crypto/tlcsprng.zig b/lib/std/crypto/tlcsprng.zig index 1e39b125f6..c23e99e2dd 100644 --- a/lib/std/crypto/tlcsprng.zig +++ b/lib/std/crypto/tlcsprng.zig @@ -48,6 +48,16 @@ const Context = struct { gimli: std.crypto.core.Gimli, }; +var install_atfork_handler = std.once(struct { + // Install the global handler only once. + // The same handler is shared among threads and is inherinted by fork()-ed + // processes. + fn do() void { + const r = std.c.pthread_atfork(null, null, childAtForkHandler); + std.debug.assert(r == 0); + } +}.do); + threadlocal var wipe_mem: []align(mem.page_size) u8 = &[_]u8{}; fn tlsCsprngFill(_: *const std.rand.Random, buffer: []u8) void { @@ -135,14 +145,8 @@ fn tlsCsprngFill(_: *const std.rand.Random, buffer: []u8) void { } fn setupPthreadAtforkAndFill(buffer: []u8) void { - const failed = std.c.pthread_atfork(null, null, childAtForkHandler) != 0; - if (failed) { - const ctx = @ptrCast(*Context, wipe_mem.ptr); - ctx.init_state = .failed; - return fillWithOsEntropy(buffer); - } else { - return initAndFill(buffer); - } + install_atfork_handler.call(); + return initAndFill(buffer); } fn childAtForkHandler() callconv(.C) void { From ec9a44b2a59a9061e1979be5268dd657cd5402fd Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Thu, 20 May 2021 15:28:59 +0200 Subject: [PATCH 3/3] std: Make atfork handler more robust The atfork handler is executed even when fork()-ing threads that have never initialized their local csprng. Handle this case gracefully instead of raising a runtime error. Fixes #8841 --- lib/std/crypto/tlcsprng.zig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/std/crypto/tlcsprng.zig b/lib/std/crypto/tlcsprng.zig index c23e99e2dd..c2cb119d4a 100644 --- a/lib/std/crypto/tlcsprng.zig +++ b/lib/std/crypto/tlcsprng.zig @@ -150,6 +150,9 @@ fn setupPthreadAtforkAndFill(buffer: []u8) void { } fn childAtForkHandler() callconv(.C) void { + // The atfork handler is global, this function may be called after + // fork()-ing threads that never initialized the CSPRNG context. + if (wipe_mem.len == 0) return; std.crypto.utils.secureZero(u8, wipe_mem); }