(Claude-generated content below) ## Fix bash shell integration use-after-free bug ### Problem After updating to the latest `main` branch, Ghostty fails to launch with bash shell integration enabled on Linux (Ubuntu 25.10). The terminal window briefly appears and then closes with an error like: ``` /bin/sh: 1: ically: not found ``` The error message varies but typically contains fragments of words (like "ically" from "automatically"). Related issue was possibly [reported in the comments of PR #9881](https://github.com/ghostty-org/ghostty/pull/9881) shortly after it was merged. ### Root Cause Analysis The bug is a **use-after-free** in the bash shell integration setup. In `setupBash()`, the `ShellCommandBuilder` is initialized with a `stackFallback` allocator: ```zig var stack_fallback = std.heap.stackFallback(4096, alloc); var cmd = internal_os.shell.ShellCommandBuilder.init(stack_fallback.get()); ``` When `cmd.toOwnedSlice()` is called, it returns a slice allocated from the `stackFallback` allocator. Since the command string (e.g., "bash --posix") easily fits within 4096 bytes, this memory is allocated on the **stack**. When `setupBash()` returns, the stack frame is deallocated, but the returned `.shell` command still points to this now-invalid stack memory. When the command is later used in `execCommand()` to build the `/bin/sh -c "..."` invocation, it reads garbage data. The garbage data often contains "ically" because it picks up fragments from nearby memory—likely from the string literal "automatically" in the comment on line 280: ```zig // being manually sourced or automatically injected (from here). ``` This is a [known footgun with Zig's stackFallback allocator](https://github.com/ziglang/zig/issues/16344) - memory allocated from the stack portion becomes invalid once the stack frame is exited. ### Bisect / Blame The bug was introduced **~5 hours ago** (earlier today): | Field | Value | |-------|-------| | **Commit** | `04fecd7c07fccad423ab1c33324a1997e142b6e2` | | **PR** | [#9881](https://github.com/ghostty-org/ghostty/pull/9881) (os/shell: introduce ShellCommandBuilder) | | **Date** | Fri Dec 12 08:59:44 2025 -0500 | | **Merged as** | `dd06d8a13` | The previous implementation in commit `c0deaaba4` used `std.mem.joinZ(alloc, " ", args.items)` which correctly allocated the result using the arena allocator. The refactor to use `ShellCommandBuilder` inadvertently changed the allocation strategy to use stack memory for the returned value. Other uses of `stackFallback` in the codebase are safe because they either: - Copy data to another allocator before the function returns (e.g., `env.put()` in `setupXdgDataDirs`) - Use the data only within the function scope (e.g., `stream_handler.zig`) ### The Fix Copy the command string to the arena allocator before returning: ```zig const cmd_str = try cmd.toOwnedSlice(); return .{ .shell = try alloc.dupeZ(u8, cmd_str) }; ``` This ensures the memory remains valid for the lifetime needed by the caller, matching the behavior of other `stackFallback` usage patterns in the codebase. ### Testing - Verified the fix on Ubuntu 25.10 with Ghostty built from source - Before fix: Terminal crashes immediately with "ically: not found" error - After fix: Terminal launches successfully with shell integration working ``` info(io_exec): shell integration automatically injected shell=.bash info(io_exec): started subcommand path=/bin/sh pid=... info(surface): surface closed addr=... # Normal close, no "abnormal process exit" ``` ### Platform Primarily affects Linux where `.shell` commands are wrapped in `/bin/sh -c "..."`. macOS uses a different execution path via `login(1)`.pull/9887/head
commit
c198c6dc69
|
|
@ -353,7 +353,11 @@ fn setupBash(
|
||||||
);
|
);
|
||||||
try env.put("ENV", integ_dir);
|
try env.put("ENV", integ_dir);
|
||||||
|
|
||||||
return .{ .shell = try cmd.toOwnedSlice() };
|
// Get the command string from the builder, then copy it to the arena
|
||||||
|
// allocator. The stackFallback allocator's memory becomes invalid after
|
||||||
|
// this function returns, so we must copy to the arena.
|
||||||
|
const cmd_str = try cmd.toOwnedSlice();
|
||||||
|
return .{ .shell = try alloc.dupeZ(u8, cmd_str) };
|
||||||
}
|
}
|
||||||
|
|
||||||
test "bash" {
|
test "bash" {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue