Skip to content

Commit cd47c76

Browse files
committed
fix-bkinterp-concurrency-issues
1 parent 3278fbd commit cd47c76

1 file changed

Lines changed: 67 additions & 43 deletions

File tree

share/brewkit/bkinterp.zig

Lines changed: 67 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@
33
// via argv[0]; ensure libexec/<cmd>'s PT_INTERP == prefix/lib/<libc>/<ldso>,
44
// re-poking only when stale (survives relocation), then exec it directly so
55
// /proc/self/exe is the real binary. No deps beyond a static musl libc.
6+
//
7+
// Concurrency: under `make -jN` the same wrapped binary is invoked many times
8+
// at once. We must NOT hold a writable fd on the target while other processes
9+
// execve it, or the kernel returns ETXTBSY. So the check is read-only, the
10+
// target is opened writable ONLY when a poke is actually required (first run /
11+
// after a move), and execve is retried on ETXTBSY to cover the brief window
12+
// where another process is mid-poke.
613
const std = @import("std");
714
const posix = std.posix;
815

@@ -24,63 +31,80 @@ pub fn main() u8 {
2431

2532
const target = std.fmt.allocPrintSentinel(a, "{s}/libexec/{s}", .{ prefix, cmd }, 0) catch return 127;
2633

27-
// --- read PT_INTERP from the target ELF ---
28-
const f = std.fs.cwd().openFile(target, .{ .mode = .read_write }) catch
29-
std.fs.cwd().openFile(target, .{}) catch return 126;
30-
var hdr: [64]u8 = undefined;
31-
_ = f.preadAll(&hdr, 0) catch return 126;
32-
const phoff = std.mem.readInt(u64, hdr[0x20..0x28], .little);
33-
const phentsize = std.mem.readInt(u16, hdr[0x36..0x38], .little);
34-
const phnum = std.mem.readInt(u16, hdr[0x38..0x3a], .little);
35-
36-
var i: u16 = 0;
34+
// --- read PT_INTERP (READ-ONLY: holding a writable fd here would race
35+
// concurrent execve()s of the same file → ETXTBSY under make -jN) ---
3736
var ioff: u64 = 0;
3837
var isz: u64 = 0;
39-
while (i < phnum) : (i += 1) {
40-
var ph: [56]u8 = undefined;
41-
_ = f.preadAll(&ph, phoff + @as(u64, i) * phentsize) catch return 126;
42-
if (std.mem.readInt(u32, ph[0..4], .little) == 3) { // PT_INTERP
43-
ioff = std.mem.readInt(u64, ph[8..16], .little);
44-
isz = std.mem.readInt(u64, ph[0x20..0x28], .little);
45-
break;
38+
var cur_raw: []u8 = &.{};
39+
{
40+
const f = std.fs.cwd().openFile(target, .{}) catch return 126;
41+
defer f.close();
42+
var hdr: [64]u8 = undefined;
43+
_ = f.preadAll(&hdr, 0) catch return 126;
44+
const phoff = std.mem.readInt(u64, hdr[0x20..0x28], .little);
45+
const phentsize = std.mem.readInt(u16, hdr[0x36..0x38], .little);
46+
const phnum = std.mem.readInt(u16, hdr[0x38..0x3a], .little);
47+
48+
var i: u16 = 0;
49+
while (i < phnum) : (i += 1) {
50+
var ph: [56]u8 = undefined;
51+
_ = f.preadAll(&ph, phoff + @as(u64, i) * phentsize) catch return 126;
52+
if (std.mem.readInt(u32, ph[0..4], .little) == 3) { // PT_INTERP
53+
ioff = std.mem.readInt(u64, ph[8..16], .little);
54+
isz = std.mem.readInt(u64, ph[0x20..0x28], .little);
55+
break;
56+
}
4657
}
47-
}
48-
if (isz == 0) return 126;
49-
const cur_raw = a.alloc(u8, isz) catch return 127;
50-
_ = f.preadAll(cur_raw, ioff) catch return 126;
58+
if (isz == 0) return 126;
59+
cur_raw = a.alloc(u8, isz) catch return 127;
60+
_ = f.preadAll(cur_raw, ioff) catch return 126;
61+
} // read-only fd closed before any poke / exec
62+
5163
const cur = std.mem.sliceTo(cur_raw, 0);
5264
const ldso = std.fs.path.basename(cur);
5365

5466
// --- find prefix/lib/*/<ldso> ---
5567
const libdir = std.fmt.allocPrint(a, "{s}/lib", .{prefix}) catch return 127;
5668
var want: ?[]u8 = null;
57-
var d = std.fs.openDirAbsolute(libdir, .{ .iterate = true }) catch return 126;
58-
defer d.close();
59-
var it = d.iterate();
60-
while (it.next() catch null) |ent| {
61-
if (ent.kind != .directory) continue;
62-
const cand = std.fmt.allocPrint(a, "{s}/{s}/{s}", .{ libdir, ent.name, ldso }) catch continue;
63-
std.fs.accessAbsolute(cand, .{}) catch continue;
64-
want = cand;
65-
break;
69+
{
70+
var dir = std.fs.openDirAbsolute(libdir, .{ .iterate = true }) catch return 126;
71+
defer dir.close();
72+
var it = dir.iterate();
73+
while (it.next() catch null) |ent| {
74+
if (ent.kind != .directory) continue;
75+
const cand = std.fmt.allocPrint(a, "{s}/{s}/{s}", .{ libdir, ent.name, ldso }) catch continue;
76+
std.fs.accessAbsolute(cand, .{}) catch continue;
77+
want = cand;
78+
break;
79+
}
6680
}
6781

68-
// --- poke if stale ---
82+
// --- poke ONLY if stale: writable fd is opened and closed immediately, so
83+
// the ETXTBSY window is limited to the rare first-run / post-move case ---
6984
if (want) |w| {
7085
if (!std.mem.eql(u8, cur, w) and w.len + 1 <= isz) {
71-
const buf = a.alloc(u8, isz) catch return 127;
72-
@memset(buf, 0);
73-
@memcpy(buf[0..w.len], w);
74-
_ = f.pwriteAll(buf, ioff) catch {};
86+
if (std.fs.cwd().openFile(target, .{ .mode = .read_write })) |wf| {
87+
defer wf.close();
88+
const buf = a.alloc(u8, isz) catch return 127;
89+
@memset(buf, 0);
90+
@memcpy(buf[0..w.len], w);
91+
_ = wf.pwriteAll(buf, ioff) catch {};
92+
} else |_| {}
7593
}
7694
}
7795

78-
// Close BEFORE exec: the kernel returns ETXTBSY if we execve a file that
79-
// is still open for writing.
80-
f.close();
81-
82-
// --- exec the real binary (execveZ returns its error set on failure only) ---
83-
const e = posix.execveZ(target, @ptrCast(argv.ptr), @ptrCast(std.os.environ.ptr));
84-
std.debug.print("bkinterp: execve {s}: {}\n", .{ target, e });
85-
return 127;
96+
// --- exec, retrying on ETXTBSY: another process may briefly hold the target
97+
// writable while poking it on first run. execveZ returns its error set
98+
// only on failure (it is noreturn on success). ---
99+
var tries: usize = 0;
100+
while (true) {
101+
const e = posix.execveZ(target, @ptrCast(argv.ptr), @ptrCast(std.os.environ.ptr));
102+
if (e == error.FileBusy and tries < 100) {
103+
tries += 1;
104+
std.Thread.sleep(std.time.ns_per_ms);
105+
continue;
106+
}
107+
std.debug.print("bkinterp: execve {s}: {}\n", .{ target, e });
108+
return 127;
109+
}
86110
}

0 commit comments

Comments
 (0)