Commit 76cec40
Ian Tay
·
2026-03-08 12:55:37 -0400 EDT
parent 11a12cc
fix(pty): isolate forked child from parent code path and heap-alloc argv Two issues in spawnPty's child (pid==0) branch: 1. `try` on allocPrint/bufPrintZ could propagate an error past the if-block, causing the forked child to fall through to the parent code path — running a second daemon on the same socket, or hitting errdefers that delete the parent's socket file. The bufPrintZ case is triggerable via a long SHELL basename. 2. The fixed-size argv_buf[64] overflowed with >63 CLI arguments. Both are fixed by extracting child setup into execChild() which returns !noreturn (exec or error), with the caller exiting on any error. The argv array is now heap-allocated to the exact size needed.
1 files changed,
+50,
-25
+50,
-25
| ... | ... | @@ -294,6 +294,48 @@ const Daemon = struct { | |
| 294 | 294 | return false; | |
| 295 | 295 | } | |
| 296 | 296 | ||
| 297 | + | /// Runs in the forked child. Either execs or returns an error (caller | |
| 298 | + | /// must exit on error -- returning would fall through to parent code). | |
| 299 | + | fn execChild(self: *Daemon) !noreturn { | |
| 300 | + | const alloc = std.heap.c_allocator; | |
| 301 | + | ||
| 302 | + | // main() set SIGPIPE to SIG_IGN, which (unlike handlers) survives | |
| 303 | + | // exec. Restore the default so the shell and its children behave | |
| 304 | + | // normally (e.g. `yes | head` should exit 141 via SIGPIPE). | |
| 305 | + | const dfl: posix.Sigaction = .{ | |
| 306 | + | .handler = .{ .handler = posix.SIG.DFL }, | |
| 307 | + | .mask = posix.sigemptyset(), | |
| 308 | + | .flags = 0, | |
| 309 | + | }; | |
| 310 | + | posix.sigaction(posix.SIG.PIPE, &dfl, null); | |
| 311 | + | ||
| 312 | + | const session_env = try std.fmt.allocPrintSentinel( | |
| 313 | + | alloc, | |
| 314 | + | "ZMX_SESSION={s}", | |
| 315 | + | .{self.session_name}, | |
| 316 | + | 0, | |
| 317 | + | ); | |
| 318 | + | _ = cross.c.putenv(session_env.ptr); | |
| 319 | + | ||
| 320 | + | if (self.command) |cmd_args| { | |
| 321 | + | const argv = try alloc.allocSentinel(?[*:0]const u8, cmd_args.len, null); | |
| 322 | + | for (cmd_args, 0..) |arg, i| { | |
| 323 | + | argv[i] = try alloc.dupeZ(u8, arg); | |
| 324 | + | } | |
| 325 | + | const err = std.posix.execvpeZ(argv[0].?, argv.ptr, std.c.environ); | |
| 326 | + | std.log.err("execvpe failed: cmd={s} err={s}", .{ cmd_args[0], @errorName(err) }); | |
| 327 | + | std.posix.exit(1); | |
| 328 | + | } | |
| 329 | + | ||
| 330 | + | const shell = util.detectShell(); | |
| 331 | + | // Use "-shellname" as argv[0] to signal login shell (traditional method) | |
| 332 | + | const login_shell = try std.fmt.allocPrintSentinel(alloc, "-{s}", .{std.fs.path.basename(shell)}, 0); | |
| 333 | + | const argv = [_:null]?[*:0]const u8{ login_shell, null }; | |
| 334 | + | const err = std.posix.execveZ(shell, &argv, std.c.environ); | |
| 335 | + | std.log.err("execve failed: err={s}", .{@errorName(err)}); | |
| 336 | + | std.posix.exit(1); | |
| 337 | + | } | |
| 338 | + | ||
| 297 | 339 | fn spawnPty(self: *Daemon) !c_int { | |
| 298 | 340 | const size = ipc.getTerminalSize(posix.STDOUT_FILENO); | |
| 299 | 341 | var ws: cross.c.struct_winsize = .{ |
| ... | ... | @@ -310,32 +352,15 @@ const Daemon = struct { | |
| 310 | 352 | } | |
| 311 | 353 | ||
| 312 | 354 | if (pid == 0) { // child pid code path | |
| 313 | - | const session_env = try std.fmt.allocPrint(self.alloc, "ZMX_SESSION={s}\x00", .{self.session_name}); | |
| 314 | - | _ = cross.c.putenv(@ptrCast(session_env.ptr)); | |
| 315 | - | ||
| 316 | - | if (self.command) |cmd_args| { | |
| 317 | - | const alloc = std.heap.c_allocator; | |
| 318 | - | var argv_buf: [64:null]?[*:0]const u8 = undefined; | |
| 319 | - | for (cmd_args, 0..) |arg, i| { | |
| 320 | - | argv_buf[i] = alloc.dupeZ(u8, arg) catch { | |
| 321 | - | std.posix.exit(1); | |
| 322 | - | }; | |
| 323 | - | } | |
| 324 | - | argv_buf[cmd_args.len] = null; | |
| 325 | - | const argv: [*:null]const ?[*:0]const u8 = &argv_buf; | |
| 326 | - | const err = std.posix.execvpeZ(argv_buf[0].?, argv, std.c.environ); | |
| 327 | - | std.log.err("execvpe failed: cmd={s} err={s}", .{ cmd_args[0], @errorName(err) }); | |
| 328 | - | std.posix.exit(1); | |
| 329 | - | } else { | |
| 330 | - | const shell = util.detectShell(); | |
| 331 | - | // Use "-shellname" as argv[0] to signal login shell (traditional method) | |
| 332 | - | var buf: [64]u8 = undefined; | |
| 333 | - | const login_shell = try std.fmt.bufPrintZ(&buf, "-{s}", .{std.fs.path.basename(shell)}); | |
| 334 | - | const argv = [_:null]?[*:0]const u8{ login_shell, null }; | |
| 335 | - | const err = std.posix.execveZ(shell, &argv, std.c.environ); | |
| 336 | - | std.log.err("execve failed: err={s}", .{@errorName(err)}); | |
| 355 | + | // In the forked child, ANY error must exit rather than propagate: | |
| 356 | + | // a returned error falls through to the parent code path below, | |
| 357 | + | // running a second daemon on the same socket (or worse, hitting | |
| 358 | + | // errdefers that delete the parent's socket file). | |
| 359 | + | execChild(self) catch |err| { | |
| 360 | + | std.log.err("child setup failed: {s}", .{@errorName(err)}); | |
| 337 | 361 | std.posix.exit(1); | |
| 338 | - | } | |
| 362 | + | }; | |
| 363 | + | unreachable; // execChild either execs or exits, never returns ok | |
| 339 | 364 | } | |
| 340 | 365 | // master pid code path | |
| 341 | 366 | self.pid = pid; |