diff --git a/.github/workflows/build-iso.yml b/.github/workflows/build-iso.yml index 349d153..f939ba8 100644 --- a/.github/workflows/build-iso.yml +++ b/.github/workflows/build-iso.yml @@ -56,7 +56,7 @@ jobs: for s in tests/smoke.sh tests/charge.sh tests/account-gate.sh \ tests/persist.sh tests/branch-gate.sh tests/startxfce-gate.sh \ tests/account.sh tests/init-session.sh \ - tests/ingot-gate.sh tests/shutdown-gate.sh; do + tests/ingot-gate.sh tests/shutdown-gate.sh tests/xprobe-gate.sh; do if [ ! -e "$s" ]; then echo "::error::missing:$s -- the workflow runs it but it is not in the tree" bad=1 diff --git a/BUGS.md b/BUGS.md index a59b4e3..f3cf8e2 100644 --- a/BUGS.md +++ b/BUGS.md @@ -135,9 +135,37 @@ both the skip check and the stamp, so a change to the probe cannot be skipped against an unchanged tree. `startxfce` already prefers it; only the build was missing. +**Hardened (2026-10-14), still not booted.** Three residual ways the same +symptom could return were closed, each with a regression test: + +- **The probe could hang.** `xprobe` set no receive timeout, so a socket that + accepts a connection and then never answers (a server still starting, or a + wedged one) blocked it forever — and the launcher with it. It now sets + `SO_RCVTIMEO`/`SO_SNDTIMEO` (`XPROBE_TIMEOUT`, default 5s) and returns a + distinct code instead of blocking. +- **The probe said only "alive / not alive".** A timeout, a permission + failure or a malformed reply all looked like "not alive", and the launcher + deleted the socket on any negative — including a live server it merely could + not speak to yet. `xprobe` now exits `0` live, `1` not-live (no socket / + refused / peer closed), `2` usage, `3` unknown (timeout / permission / + auth / malformed); `startxfce` only clears the socket on `1` and leaves it + alone on `3`. +- **A rootfs with no probe at all.** The bare socket-existence fallback *is* + the bug. With no probe, `startxfce` now consults `/proc/net/unix` + (busybox-safe, exact pathname match) and treats "bound" as live, "absent" + as dead, and "cannot read" as unknown — never as live. + +Regression tests: `tests/xprobe-gate.sh` drives the real compiled binary +against a synthetic X server (no third-party deps, skips cleanly without a C +compiler) and is wired into `tests/branch-gate.sh` and the ISO workflow; +`tests/startxfce-gate.sh` gained cases 16-21 for the no-probe fallback, live +reuse, the unknown-leaves-the-socket rule, the bounded startup timeout, +`XPROBE_TIMEOUT` passthrough and a probe stuck at "not live". + **Still open because:** "built" is not "verified". Needs a boot with the persistence disk: start a desktop, reboot, and `startxfce` must start a fresh -server rather than announce the stale one. +server rather than announce the stale one. For this change a full ISO build +and a VM reboot were **not** run; verification stopped at the passing gates. --- diff --git a/HANDOFF.md b/HANDOFF.md index f39e13a..fa262f4 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -315,7 +315,9 @@ logs in as the wizard user, `startxfce` (no `sudo`) paints XFCE, and **Firefox and nmap run in the session** — bugs 19, 22 and 23 closed. That boot was a VMware guest, which also settles the VMware framebuffer — bug 21 closed. What remains is the edges, not "does it work": a persisted `/tmp` making the second -boot reuse a dead display (bug 24, fixed in the tree, needs a two-boot test) and +boot reuse a dead display (bug 24 — the build integration is fixed in the tree +and the residual hang / false-live paths are now hardened, with real-binary and +launcher regression gates, but the two-boot test is still owed) and `sudo shutdown`/`sudo reboot` (bug 18, fixed in the tree, needs a boot). Order of the remaining work — 1–3 are done (boot-verified 2026-10-11): @@ -546,6 +548,8 @@ Carried over from the pacman work, still true: - `iso/build.sh` **gui stage only** (~895-995) — apt package list at 916, unpack, loader copy, gates. - `tests/startxfce-gate.sh` — CI runs it every push; keep it green. +- `tests/xprobe-gate.sh` — drives the real `xprobe` binary against a synthetic + X server; runs via `tests/branch-gate.sh` and the ISO workflow. Keep it green. **Not yours — do not restructure:** @@ -580,6 +584,8 @@ src/ copper-sh: main.c, builtins.c, builtins.h tests/smoke.sh 19-assertion shell smoke test tests/account-gate.sh asserts the shipped account/answer path tests/shutdown-gate.sh proves /sbin/shutdown maps flags to the right applet +tests/startxfce-gate.sh 21 cases: the launcher's display states and exit codes +tests/xprobe-gate.sh the real xprobe binary vs a synthetic X server .github/workflows/build-iso.yml per-stage CI steps + workspace cache ``` diff --git a/iso/startxfce.sh b/iso/startxfce.sh index 96ed98a..416de96 100755 --- a/iso/startxfce.sh +++ b/iso/startxfce.sh @@ -36,9 +36,11 @@ # "never opened the display" about a server that is working. Learning which # display the server chose means parsing its log or querying it, and POSIX # sh can do neither. -# - Once xdpyinfo exists, the wait loop runs it every iteration, so a slow -# probe multiplies the timeout by its own runtime. Harmless now -- with no -# probe the loop only stats a socket -- and worth tightening when it lands. +# - The wait loop runs the probe every iteration, so a slow probe adds its +# own runtime to each try. The probe is bounded now: xprobe gives up after +# XPROBE_TIMEOUT seconds and reports the display unknown instead of +# blocking, so the loop always ends -- but a machine where each probe takes +# the full timeout still stretches the wait by that much. # - The timeout is 10s where sleep accepts fractions and 100s where it does # not, because of the fallback in the wait loop. Same image, a tenfold # difference in how long a failure appears to hang. @@ -80,7 +82,10 @@ Environment, all optional: COPPER_XLOG where the X server logs (default /tmp/xorg.log) COPPER_SESSIONLOG where XFCE's output goes (default /tmp/session.log) COPPER_XPROBE X client used to check whether a display really - answers (default /usr/bin/xdpyinfo) + answers (default: the first of /usr/bin/xprobe and + /usr/bin/xdpyinfo; the kernel's socket table if neither) + XPROBE_TIMEOUT seconds xprobe waits for a display to answer before it + reports it unknown and the launcher moves on (default 5) COPPER_XORGCONF where the generated input config is written (default /etc/X11/xorg.conf; falls back to /tmp/xorg.conf when the login cannot write there) @@ -150,25 +155,70 @@ done # socket and reads the geometry back, needing nothing but libc. xdpyinfo would do # the same job but arrives as part of x11-utils, which is another package to # build and stage before the display can be checked. Either is accepted, and the -# probe found first is the one used; the socket test is the fallback when neither -# is installed, and that fallback is the weaker of the three. +# probe found first is the one used. +# +# xprobe's exit code is part of its contract with this script, because it decides +# what is safe to do next (the same codes are written down in xprobe.c): +# +# 0 live the display answered the handshake; reuse it +# 1 not live no socket, or the connection was refused; the file is stale and +# may be cleared before a fresh server starts +# 2 usage xprobe was called wrongly; never produced from here +# 3 unknown something was reached but would not answer -- a timeout, a +# permission or authorisation failure, a malformed reply. NOT +# proof the socket is stale, so nothing is removed on this answer +# +# When no probe is installed the socket file is still not proof. The kernel's +# socket table is asked instead: /proc/net/unix lists a socket a process is +# bound to and forgets one whose process is gone, which is exactly the +# difference a bare file cannot show. If even that is unreadable the answer is +# unknown, never live, so a missing probe can never make a stale socket look +# alive. XPROBE=${COPPER_XPROBE:-} if [ -z "$XPROBE" ]; then for cand in /usr/bin/xprobe /usr/bin/xdpyinfo; do if [ -x "$cand" ]; then XPROBE=$cand; break; fi done fi +XPROBE_TIMEOUT=${XPROBE_TIMEOUT:-5} -display_live() { - [ -S "$XSOCKDIR/X$DISPLAY_NUM" ] || return 1 +# Is a process actually bound to this unix socket path? +# 0 bound, 1 not bound, 2 cannot tell. +socket_bound() { + [ -r /proc/net/unix ] || return 2 + _sock_want=$1 + # The path is the eighth column of /proc/net/unix. A line with no path (the + # header, or a socket that was never bound to one) leaves _f8 empty and + # matches nothing, which is right. + while read -r _f1 _f2 _f3 _f4 _f5 _f6 _f7 _f8; do + [ "$_f8" = "$_sock_want" ] && return 0 + done < /proc/net/unix + return 1 +} + +# The whole liveness decision, in one place, so the reuse check and the wait +# loop cannot drift apart. Prints exactly one word -- live, dead, or unknown -- +# so callers read it with $(display_state) and no diagnostic can leak into the +# answer. +display_state() { + _ds_sock="$XSOCKDIR/X$DISPLAY_NUM" + [ -S "$_ds_sock" ] || { echo dead; return 0; } if [ -n "$XPROBE" ] && [ -x "$XPROBE" ]; then - DISPLAY=":$DISPLAY_NUM" "$XPROBE" >/dev/null 2>&1 && return 0 - # The socket is there and nothing answers it: a stale socket from a - # session that did not clean up. Treated as no display, so a fresh - # server gets a chance to replace it. - return 1 + DISPLAY=":$DISPLAY_NUM" XPROBE_SOCKET_DIR="$XSOCKDIR" \ + XPROBE_TIMEOUT="$XPROBE_TIMEOUT" "$XPROBE" >/dev/null 2>&1 + case "$?" in + 0) echo live ;; + 1) echo dead ;; + *) echo unknown ;; + esac + return 0 fi - return 0 + socket_bound "$_ds_sock" + case "$?" in + 0) echo live ;; + 1) echo dead ;; + *) echo unknown ;; + esac } # What is installed decides this before anything else, and independently of @@ -269,7 +319,16 @@ if [ -n "$missing" ]; then fi fi -if display_live; then +# If the caller wanted a probe and none is installed, say so once, rather than +# letting the weaker socket-table fallback pass silently for the life of the +# image. +if [ -z "$XPROBE" ] || [ ! -x "$XPROBE" ]; then + say "no X liveness probe (looked for \$COPPER_XPROBE, /usr/bin/xprobe," + say " /usr/bin/xdpyinfo); checking the kernel's socket table instead." +fi + +disp_state=$(display_state) +if [ "$disp_state" = live ]; then say "an X server is already on :$DISPLAY_NUM, using it" else if [ "$have_xorg" != 1 ]; then @@ -278,12 +337,24 @@ else fi say "starting $XORG on :$DISPLAY_NUM, log in $XLOG" - # display_live() rejected this socket, so nothing is listening on it. Some - # servers refuse to bind over a socket file that already exists, so clear - # it before starting. - if [ -S "$XSOCKDIR/X$DISPLAY_NUM" ] && [ -w "$XSOCKDIR" ]; then - rm -f "$XSOCKDIR/X$DISPLAY_NUM" 2>/dev/null - fi + # Clear the socket only when the display is known to be dead. "dead" is + # positive evidence -- no socket at all, or a connection that was refused. + # "unknown" means something was reached but would not answer, which may be a + # server still starting or one this user may not talk to; removing its + # socket would break it, so it is left alone and the start proceeds. + case "$disp_state" in + dead) + # Some servers refuse to bind over a socket file that already + # exists, so a socket known to be stale is cleared first. + if [ -S "$XSOCKDIR/X$DISPLAY_NUM" ] && [ -w "$XSOCKDIR" ]; then + rm -f "$XSOCKDIR/X$DISPLAY_NUM" 2>/dev/null + fi + ;; + unknown) + say "a socket exists on :$DISPLAY_NUM but the display did not answer;" + say " leaving it in place in case a server is still starting." + ;; + esac # Input devices, and the config that names them. # @@ -451,7 +522,7 @@ else # identical if all you do is count to a limit. i=0 while [ "$i" -lt "$WAIT_TRIES" ]; do - if display_live; then + if [ "$(display_state)" = live ]; then break fi if ! kill -0 "$xpid" 2>/dev/null; then @@ -463,7 +534,7 @@ else sleep 0.1 2>/dev/null || sleep 1 done - if ! display_live; then + if [ "$(display_state)" != live ]; then say "the X server is running but never opened :$DISPLAY_NUM." say "its log says:" sed 's/^/ /' "$XLOG" 2>/dev/null diff --git a/iso/x11/probe/xprobe.c b/iso/x11/probe/xprobe.c index 5430003..7df607b 100644 --- a/iso/x11/probe/xprobe.c +++ b/iso/x11/probe/xprobe.c @@ -17,6 +17,27 @@ * * Output is one line per fact, "name=value", so a test can grep for a value * rather than match prose. + * + * Exit codes -- a contract with startxfce, which decides what is safe to do + * next from them. Everything used to be 1, which threw away the one + * distinction that matters on a machine whose /tmp persists: whether the + * socket refused the connection (it is stale and may be cleared) or was + * reached but would not answer (it may be a server still starting, or one we + * are not permitted to talk to, and must be left alone). + * + * 0 live the handshake and the GetGeometry round trip both completed. + * 1 not live no socket, or the connection was refused / the peer closed + * without answering. Positive evidence nothing usable is + * listening; a stale socket file. + * 2 usage a bad or missing DISPLAY, or an unknown option. Never + * produced by startxfce, which always passes ":$DISPLAY_NUM". + * 3 unknown something was reached but the answer did not arrive or did + * not parse: a timeout, a permission or authorisation refusal, + * a malformed reply. NOT proof the socket is stale. + * + * Every socket operation is bounded by SO_RCVTIMEO/SO_SNDTIMEO, so a listener + * that accepts the connection and then says nothing returns "unknown" after + * XPROBE_TIMEOUT seconds instead of hanging the caller forever. */ #include @@ -25,6 +46,7 @@ #include #include #include +#include #include #include @@ -32,6 +54,17 @@ #define X_TCP_PORT 6000 +/* Exit codes, restated here so the code reads the same as the header. */ +#define XP_LIVE 0 +#define XP_NOTLIVE 1 +#define XP_USAGE 2 +#define XP_UNKNOWN 3 + +/* Seconds a connect, a write, or a read may block before the display is called + unknown. Overridable through XPROBE_TIMEOUT so a test can make a hang fail + fast, and so a slow machine can be given more room than the caller's loop. */ +#define XP_DEFAULT_TIMEOUT 5 + struct setup_request { uint8_t byte_order; /* 0x6c = 'l', little endian */ uint8_t pad0; @@ -110,8 +143,10 @@ static int read_all(int fd, void *p, size_t n) { char *c = p; while (n) { ssize_t r = read(fd, c, n); - if (r == 0) - return -1; /* closed: truncated reply */ + if (r == 0) { + errno = EPIPE; /* closed: truncated reply, peer is gone */ + return -1; + } if (r < 0) { if (errno == EINTR) continue; @@ -123,6 +158,32 @@ static int read_all(int fd, void *p, size_t n) { return 0; } +/* Bound every socket operation, so a listener that accepts the connection and + then says nothing cannot hang the caller. SO_RCVTIMEO/SO_SNDTIMEO make the + blocking calls fail with EAGAIN after tv instead of waiting forever. */ +static int set_timeout(int fd, int seconds) { + struct timeval tv; + + tv.tv_sec = seconds; + tv.tv_usec = 0; + if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof tv) < 0) + return -1; + if (setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof tv) < 0) + return -1; + return 0; +} + +/* Turn the errno left by a failed write/read into an exit code. A peer that + closed, or a connection that was refused, is positive evidence nothing + usable is listening; a timeout or a permission error is not, and must not be + read as permission to clear the socket. */ +static int io_failure_code(void) { + if (errno == EPIPE || errno == ECONNRESET || errno == ECONNREFUSED || + errno == ENOENT) + return XP_NOTLIVE; + return XP_UNKNOWN; +} + static uint32_t rd32(const unsigned char *p) { return (uint32_t)p[0] | ((uint32_t)p[1] << 8) | ((uint32_t)p[2] << 16) | ((uint32_t)p[3] << 24); @@ -149,8 +210,10 @@ static uint16_t rd16(const unsigned char *p) { int main(int argc, char **argv) { const char *display_arg = NULL; const char *sockdir; + const char *tmo; int use_unix = 1; int display_num = -1; + int timeout = XP_DEFAULT_TIMEOUT; struct sockaddr_un sun; struct setup_request req; struct setup_reply rep; @@ -167,7 +230,7 @@ int main(int argc, char **argv) { display_arg = argv[i]; } else { fprintf(stderr, "xprobe: unknown option %s\n", argv[i]); - return 2; + return XP_USAGE; } } if (!display_arg) @@ -179,9 +242,19 @@ int main(int argc, char **argv) { sockdir = getenv("XPROBE_SOCKET_DIR"); if (!sockdir || !*sockdir) sockdir = "/tmp/.X11-unix"; + /* How long any single socket operation may block. Bad values are ignored + rather than fatal: a misconfigured timeout must not stop the probe from + doing its job. */ + tmo = getenv("XPROBE_TIMEOUT"); + if (tmo && *tmo) { + char *end; + long v = strtol(tmo, &end, 10); + if (*end == '\0' && v > 0 && v <= 3600) + timeout = (int)v; + } if (!display_arg || !*display_arg) { fprintf(stderr, "xprobe: no DISPLAY, nothing to connect to\n"); - return 2; + return XP_USAGE; } /* DISPLAY is ":N", ":N.S", "host:N", or "unix:N". Only :N and unix:N are @@ -200,7 +273,7 @@ int main(int argc, char **argv) { if (!colon) { fprintf(stderr, "xprobe: DISPLAY=%s has no display number\n", display_arg); - return 2; + return XP_USAGE; } hl = (size_t)(colon - display_arg); if (hl >= sizeof host) @@ -212,13 +285,13 @@ int main(int argc, char **argv) { if (host[0] && strcmp(host, "unix") != 0) { fprintf(stderr, "xprobe: DISPLAY=%s names a host, and this has no " "network stack to reach it\n", display_arg); - return 2; + return XP_USAGE; } display_num = (int)strtol(rest, NULL, 10); if (display_num < 0) { fprintf(stderr, "xprobe: DISPLAY=%s has no display number\n", display_arg); - return 2; + return XP_USAGE; } /* A screen suffix (.S) is legal and ignored here: the handshake reports every screen and picking between them is a client's job. */ @@ -229,8 +302,13 @@ int main(int argc, char **argv) { fd = socket(AF_UNIX, SOCK_STREAM, 0); if (fd < 0) { fprintf(stderr, "xprobe: socket: %s\n", strerror(errno)); - return 1; + return XP_UNKNOWN; } + /* Bound every operation on this socket before it is used, including the + handshake reads below, so nothing here can block the caller forever. */ + if (set_timeout(fd, timeout) < 0) + fprintf(stderr, "xprobe: set_timeout: %s (continuing)\n", + strerror(errno)); memset(&sun, 0, sizeof sun); sun.sun_family = AF_UNIX; /* The abstract socket is /tmp/.X11-unix/XN; the filesystem path is what a @@ -241,7 +319,7 @@ int main(int argc, char **argv) { fprintf(stderr, "xprobe: socket path for display %d does not fit in " "sun_path\n", display_num); close(fd); - return 2; + return XP_USAGE; } if (connect(fd, (struct sockaddr *)&sun, sizeof sun) < 0) { int saved = errno; @@ -254,7 +332,7 @@ int main(int argc, char **argv) { fprintf(stderr, "xprobe: socket path for display %d does not fit " "in sun_path\n", display_num); close(fd); - return 2; + return XP_USAGE; } if (connect(fd, (struct sockaddr *)&sun, (socklen_t)(sizeof(sun.sun_family) + 1 + @@ -264,13 +342,22 @@ int main(int argc, char **argv) { the two: "No such file" means no server ever got here, and "Connection refused" means one did and is now gone. */ char path[sizeof sun.sun_path]; + int ab = errno; snprintf(path, sizeof path, "%s/X%d", sockdir, display_num); fprintf(stderr, "xprobe: connect %s: %s\n", path, strerror(saved)); fprintf(stderr, "xprobe: (abstract) %s: %s\n", sun.sun_path + 1, - strerror(errno)); + strerror(ab)); close(fd); - return 1; + /* "No such file" on both means no server ever used this display; + "Connection refused" means one did and is gone. Either is proof + the socket is not live. A permission error is neither, and is + reported as unknown so the caller does not clear a socket it + cannot see into. */ + if (saved == ENOENT || saved == ECONNREFUSED || + ab == ENOENT || ab == ECONNREFUSED) + return XP_NOTLIVE; + return XP_UNKNOWN; } } printf("connected=yes\n"); @@ -284,15 +371,26 @@ int main(int argc, char **argv) { req.protocol_major = 11; req.protocol_minor = 0; if (write_all(fd, &req, sizeof req) < 0) { + int code = io_failure_code(); + fprintf(stderr, "xprobe: write setup: %s\n", strerror(errno)); close(fd); - return 1; + return code; } if (read_all(fd, &rep, sizeof rep) < 0) { - fprintf(stderr, "xprobe: server closed during the handshake\n"); + int code = io_failure_code(); + + /* A timeout is not a closed connection, and saying so is the useful + part of the report: "server closed" would send someone looking for + a crash that never happened. */ + if (errno == EAGAIN || errno == EWOULDBLOCK) + fprintf(stderr, "xprobe: timed out waiting for the server's reply " + "(XPROBE_TIMEOUT seconds)\n"); + else + fprintf(stderr, "xprobe: server closed during the handshake\n"); close(fd); - return 1; + return code; } printf("handshake.major=%u\n", rd16((unsigned char *)&rep.protocol_major)); printf("handshake.minor=%u\n", rd16((unsigned char *)&rep.protocol_minor)); @@ -308,7 +406,8 @@ int main(int argc, char **argv) { fprintf(stderr, "xprobe: the server wants authorisation; start it " "with -auth for a real client\n"); close(fd); - return 1; + /* A live server we are not allowed to talk to. Not stale. */ + return XP_UNKNOWN; } printf("handshake.status=failed\n"); want = extra < sizeof reason ? extra : (uint32_t)sizeof reason; @@ -318,7 +417,9 @@ int main(int argc, char **argv) { else fprintf(stderr, "xprobe: server refused the connection\n"); close(fd); - return 1; + /* The server answered, so something is there; it just would not let + this client in. Unknown, not dead: do not clear the socket. */ + return XP_UNKNOWN; } printf("handshake.status=success\n"); @@ -326,7 +427,9 @@ int main(int argc, char **argv) { if (read_all(fd, &body, sizeof body) < 0) { fprintf(stderr, "xprobe: truncated setup reply\n"); close(fd); - return 1; + /* The handshake already succeeded, so a server is here; it just did + not finish talking. Unknown, so the socket is not cleared. */ + return XP_UNKNOWN; } { /* The setup reply is vendor string, then pixmap formats, then the @@ -360,7 +463,7 @@ int main(int argc, char **argv) { "declared, %u needed for vendor, formats and one " "screen)\n", rest, want); close(fd); - return 1; + return XP_UNKNOWN; } memset(vendorbuf, 0, sizeof vendorbuf); if (read_all(fd, vendorbuf, vendor_pad) < 0 || @@ -368,7 +471,7 @@ int main(int argc, char **argv) { read_all(fd, screenbuf, sizeof screenbuf) < 0) { fprintf(stderr, "xprobe: truncated setup reply\n"); close(fd); - return 1; + return XP_UNKNOWN; } /* Drain to the end of the reply, so the next read on this socket is the GetGeometry answer and not the tail of the handshake. */ @@ -376,7 +479,7 @@ int main(int argc, char **argv) { if (left && skip_bytes(fd, left) < 0) { fprintf(stderr, "xprobe: truncated setup reply\n"); close(fd); - return 1; + return XP_UNKNOWN; } vendor = (const char *)vendorbuf; @@ -442,9 +545,11 @@ int main(int argc, char **argv) { g.drawable = root; if (write_all(fd, &g, sizeof g) < 0 || read_all(fd, &gr, sizeof gr) < 0) { + int code = io_failure_code(); + fprintf(stderr, "xprobe: GetGeometry did not come back\n"); close(fd); - return 1; + return code; } /* Byte 0 is the packet kind: 1 is a reply, 0 is an error whose code is in byte 1. A sequence number is never zero, so testing @@ -457,7 +562,7 @@ int main(int argc, char **argv) { fprintf(stderr, "xprobe: server sent error %u for GetGeometry\n", gr.depth); close(fd); - return 1; + return XP_UNKNOWN; } printf("getgeometry.status=ok\n"); printf("getgeometry.sequence=%u\n", rd16((unsigned char *)&gr.sequence)); @@ -471,5 +576,5 @@ int main(int argc, char **argv) { printf("probe=ok\n"); close(fd); - return 0; + return XP_LIVE; } \ No newline at end of file diff --git a/tests/branch-gate.sh b/tests/branch-gate.sh index c02421d..e879a61 100755 --- a/tests/branch-gate.sh +++ b/tests/branch-gate.sh @@ -1,13 +1,15 @@ #!/usr/bin/env bash # Run this branch's tests, and be honest about which ones did not run. # -# Eight gates: smoke.sh always; startxfce-gate.sh, charge.sh, account-gate.sh, +# Nine gates: smoke.sh always; startxfce-gate.sh, charge.sh, account-gate.sh, # persist.sh and shutdown-gate.sh need root, because they exercise sockets, # staged rootfs trees, the hotfix database, a mount namespace and a private # /sbin; init-session.sh needs root because it calls setuid. ingot-gate.sh does # not — its install lands in a scratch INGOT_ROOT and its repo is served on -# localhost — so it runs unprivileged too. CI runs this same file with sudo, so -# a green build there means every gate ran and none were skipped. +# localhost — so it runs unprivileged too. xprobe-gate.sh needs a C compiler +# and python3, which CI and the build VM have, and skips cleanly where neither +# exists. CI runs this same file with sudo, so a green build there means every +# gate ran and none were skipped. # # This file used to carry known-failure classifications: the CI workflow came # over from `untested`, and on the old `gui` tree two of its tests failed @@ -89,6 +91,28 @@ else printf ' (%s checks passed; the failures are the lines above)\n' "$oks" fi +# --------------------------------------------------------------------------- +echo +echo "--- tests/xprobe-gate.sh: the real xprobe binary vs a synthetic X server ---" +# Compiles the actual iso/x11/probe/xprobe.c and drives it through every socket +# state (no socket, stale file, mute listener, live handshake, usage errors, +# bounded timeout), asserting the 0/1/2/3 exit-code contract, plus a static +# check that iso/build.sh stages the result as /usr/bin/xprobe. Needs a C +# compiler and python3 -- both present in CI and on the build VM. Skips cleanly +# on a host with neither (the Windows box this tree is maintained on). +if out=$(bash tests/xprobe-gate.sh 2>&1); then + case "$out" in + *SKIPPED*) + record xprobe-gate.sh skipped "no C compiler on this host" ;; + *) + n=$(printf '%s' "$out" | grep -c '^ ok' || true) + record xprobe-gate.sh ok "$n checks" ;; + esac +else + record xprobe-gate.sh bad "a real-xprobe check failed" + printf '%s\n' "$out" | grep -E '^ FAIL|^ ' | sed 's/^/ /' +fi + # --------------------------------------------------------------------------- echo echo "--- tests/charge.sh: covers hotfix charge/rollback ---" diff --git a/tests/startxfce-gate.sh b/tests/startxfce-gate.sh index ccdf6d8..450a1ec 100755 --- a/tests/startxfce-gate.sh +++ b/tests/startxfce-gate.sh @@ -92,6 +92,12 @@ hasnt() { # empty for cases 1, 2 and 7, where "is a server already there" must be false. PROBE=$TMP/no-such-xdpyinfo +# A socket directory of its own for the probe-driven cases (16 onward). See case +# 16 for why: a stub's python child can keep an unlinked socket's pathname +# listed in /proc/net/unix, and the socket-table fallback would then read a path +# another case bound as still live. +PSOCK="$TMP/probe-cases/.X11-unix" + run_sut() { # The three settings are saved before the shift, because shift renumbers the # positional parameters and they are gone afterwards. @@ -588,6 +594,241 @@ APPS_MANIFEST= eq " no prompt, straight to the X check" 20 "$rc" hasnt " never asked about components" "do you want to install" "$out" +echo +echo "=== case 16: a stale socket with NO probe is still not trusted (socket table) ===" +# The image ships with xprobe built in, but an older image -- or a rootfs that +# failed to stage it -- has neither xprobe nor xdpyinfo. A missing probe must +# not silently turn a stale socket into a live server, so the kernel's socket +# table is asked instead: /proc/net/unix lists a socket a process is bound to +# and forgets one whose process is gone. COPPER_XPROBE is deliberately left +# pointing at a binary that does not exist, and a bound-then-closed socket must +# read as dead, be cleared, and be replaced. +# +# These probe cases use a socket path of their own. A stub's python child can +# outlive the stub shell it was launched from, and the kernel keeps listing an +# unlinked-but-open socket's pathname in /proc/net/unix -- so a path an earlier +# case bound would make this case's socket-table answer read "live" for a file +# that refuses connections. A directory nothing else has touched keeps the +# fallback honest. +reset +mkdir -p "$PSOCK" +python3 -c "import socket,sys +s = socket.socket(socket.AF_UNIX) +s.bind('$PSOCK/X0') +s.close()" +if [ -S "$PSOCK/X0" ]; then + ok " set up: a stale socket file exists with nothing listening on it" +else + bad " set up: a stale socket" "could not create one" +fi +cat > "$TMP/Xorg" < "$TMP/startxfce4"; chmod +x "$TMP/startxfce4" +out=$(COPPER_XORG="$TMP/Xorg" COPPER_STARTXFCE4="$TMP/startxfce4" \ + COPPER_XLOG="$TMP/xorg.log" COPPER_SESSIONLOG="$TMP/session.log" \ + COPPER_XPROBE="$PROBE" COPPER_XORGCONF="$TMP/xorg.conf" \ + COPPER_XFCE_APPS="$TMP/no-apps" COPPER_INGOT=ingot COPPER_SUDO="" \ + XSOCKDIR="$PSOCK" WAIT_TRIES=80 sh "$SUT" 2>&1); rc=$? +pkill -f "$TMP/Xorg" 2>/dev/null; pkill -f "$PSOCK" 2>/dev/null +eq " a missing probe is not false success; a fresh server was started" 0 "$rc" +has " says the probe is missing and names the fallback" "no X liveness probe" "$out" +has " names the socket-table fallback" "checking the kernel's socket table" "$out" +has " started its own server" "starting $TMP/Xorg" "$out" +has " the display came up (so the stale socket was cleared)" "X server is up" "$out" + +echo +echo "=== case 17: a probe that answers live reuses the display ===" +# The probe's exit codes are the whole contract: 0 means live. With a real +# listener present and a probe saying "live", the launcher reuses the display +# instead of starting a second server that would fight for the socket. +reset +mkdir -p "$PSOCK" +python3 -c "import socket,time +s = socket.socket(socket.AF_UNIX) +s.bind('$PSOCK/X0') +s.listen(1) +time.sleep(60)" & +spid=$! +sleep 1 +printf '#!/bin/sh\necho "ERROR: this stub must not run"\nexit 99\n' > "$TMP/Xorg" +chmod +x "$TMP/Xorg" +printf '#!/bin/sh\necho "DISPLAY=$DISPLAY" > %s/session.env\n' "$TMP" > "$TMP/startxfce4" +chmod +x "$TMP/startxfce4" +cat > "$TMP/probe" <<'PROBE_EOF' +#!/bin/sh +exit 0 +PROBE_EOF +chmod +x "$TMP/probe" +out=$(COPPER_XORG="$TMP/Xorg" COPPER_STARTXFCE4="$TMP/startxfce4" \ + COPPER_XLOG="$TMP/xorg.log" COPPER_SESSIONLOG="$TMP/session.log" \ + COPPER_XPROBE="$TMP/probe" COPPER_XORGCONF="$TMP/xorg.conf" \ + XSOCKDIR="$PSOCK" WAIT_TRIES=5 sh "$SUT" 2>&1); rc=$? +kill "$spid" 2>/dev/null +eq " exit code is 0" 0 "$rc" +has " reused the running server" "already on" "$out" +hasnt " did not start a second server" "starting $TMP/Xorg" "$out" +if [ -s "$TMP/session.env" ]; then + ok " the session ran on the existing server" +else + bad " the session ran on the existing server" "session.env missing" +fi + +echo +echo "=== case 18: an unanswered ('unknown') probe does NOT clear the socket ===" +# Exit 3 is the contract for "something is there but will not answer": a server +# still starting, one this user may not speak to, or a kernel that will not +# report. None of that is proof the socket is stale, so the launcher leaves the +# file alone and gives a fresh server its chance anyway. +reset +mkdir -p "$PSOCK" +python3 -c "import socket,sys +s = socket.socket(socket.AF_UNIX) +s.bind('$PSOCK/X0') +s.close()" +cat > "$TMP/probe" <<'PROBE_EOF' +#!/bin/sh +exit 3 +PROBE_EOF +chmod +x "$TMP/probe" +# The stub server dies immediately. The launcher must not have deleted the +# socket before starting it: the "unknown" answer forbids that. +printf '#!/bin/sh\nexit 0\n' > "$TMP/Xorg"; chmod +x "$TMP/Xorg" +printf '#!/bin/sh\nexit 0\n' > "$TMP/startxfce4"; chmod +x "$TMP/startxfce4" +out=$(COPPER_XORG="$TMP/Xorg" COPPER_STARTXFCE4="$TMP/startxfce4" \ + COPPER_XLOG="$TMP/xorg.log" COPPER_SESSIONLOG="$TMP/session.log" \ + COPPER_XPROBE="$TMP/probe" COPPER_XORGCONF="$TMP/xorg.conf" \ + XSOCKDIR="$PSOCK" WAIT_TRIES=5 sh "$SUT" 2>&1); rc=$? +eq " an unknown display still fails cleanly, not a hang or false success" 23 "$rc" +has " says it is leaving the socket in place" "leaving it in place" "$out" +has " blames the display that did not answer" "did not answer" "$out" +if [ -S "$PSOCK/X0" ]; then + ok " the socket survived the unknown answer" +else + bad " the socket survived the unknown answer" "it was removed despite 'unknown'" +fi + +echo +echo "=== case 19: a server that never answers times out instead of hanging ===" +# The server holds the socket but its handshake never completes -- the 'mute' +# state xprobe reports as exit 3. The launcher must neither hang nor claim +# success: it polls, the probe keeps saying unknown, and the bounded wait runs +# out with exit 22 naming the display. +reset +mkdir -p "$PSOCK" +python3 -c "import socket,sys +s = socket.socket(socket.AF_UNIX) +s.bind('$PSOCK/X0') +s.close()" +cat > "$TMP/probe" <<'PROBE_EOF' +#!/bin/sh +exit 3 +PROBE_EOF +chmod +x "$TMP/probe" +cat > "$TMP/Xorg" < "$TMP/startxfce4"; chmod +x "$TMP/startxfce4" +out=$(COPPER_XORG="$TMP/Xorg" COPPER_STARTXFCE4="$TMP/startxfce4" \ + COPPER_XLOG="$TMP/xorg.log" COPPER_SESSIONLOG="$TMP/session.log" \ + COPPER_XPROBE="$TMP/probe" COPPER_XORGCONF="$TMP/xorg.conf" \ + XSOCKDIR="$PSOCK" WAIT_TRIES=5 sh "$SUT" 2>&1); rc=$? +pkill -f "$TMP/Xorg" 2>/dev/null; pkill -f "$PSOCK" 2>/dev/null +eq " bounded wait, nonzero exit, no hang" 22 "$rc" +has " says the display never opened" "never opened" "$out" +has " still kept the socket for the unknown display" "leaving it in place" "$out" + +echo +echo "=== case 20: XPROBE_TIMEOUT reaches the probe ===" +# display_state runs the probe with the caller's XPROBE_TIMEOUT. The probe +# records what it was given; if it saw the default 5 instead of 7, the wiring +# is wrong and the bounded-wait guarantee silently returns to the default. +reset +mkdir -p "$PSOCK" +python3 -c "import socket,time +s = socket.socket(socket.AF_UNIX) +s.bind('$PSOCK/X0') +s.listen(1) +time.sleep(60)" & +spid=$! +sleep 1 +printf '#!/bin/sh\necho "ERROR: this stub must not run"\nexit 99\n' > "$TMP/Xorg" +chmod +x "$TMP/Xorg" +printf '#!/bin/sh\nexit 0\n' > "$TMP/startxfce4"; chmod +x "$TMP/startxfce4" +cat > "$TMP/probe" < "$TMP/probe.env" +exit 0 +PROBE_EOF +chmod +x "$TMP/probe" +out=$(COPPER_XORG="$TMP/Xorg" COPPER_STARTXFCE4="$TMP/startxfce4" \ + COPPER_XLOG="$TMP/xorg.log" COPPER_SESSIONLOG="$TMP/session.log" \ + COPPER_XPROBE="$TMP/probe" COPPER_XORGCONF="$TMP/xorg.conf" \ + XSOCKDIR="$PSOCK" WAIT_TRIES=5 XPROBE_TIMEOUT=7 \ + sh "$SUT" 2>&1); rc=$? +kill "$spid" 2>/dev/null +eq " exit code is 0" 0 "$rc" +has " reused the running server" "already on" "$out" +if grep -q '^timeout=7$' "$TMP/probe.env" 2>/dev/null; then + ok " the probe was given XPROBE_TIMEOUT=7" +else + bad " the probe was given XPROBE_TIMEOUT=7" "probe saw: $(cat "$TMP/probe.env" 2>/dev/null)" +fi + +echo +echo "=== case 21: a probe stuck at 'not live' cannot wedge the launcher ===" +# A probe that answers 1 to everything reports a healthy server as dead. The +# launcher must not hang, must not claim success, and must end with a nonzero +# exit naming the display. The Xorg stub binds without unlinking: reaching exit +# 22 also proves the stale socket was cleared (a leftover file would make the +# bind fail and the run end as exit 23 instead). +reset +mkdir -p "$PSOCK" +python3 -c "import socket,sys +s = socket.socket(socket.AF_UNIX) +s.bind('$PSOCK/X0') +s.close()" +cat > "$TMP/probe" <<'PROBE_EOF' +#!/bin/sh +exit 1 +PROBE_EOF +chmod +x "$TMP/probe" +cat > "$TMP/Xorg" < "$TMP/startxfce4"; chmod +x "$TMP/startxfce4" +out=$(COPPER_XORG="$TMP/Xorg" COPPER_STARTXFCE4="$TMP/startxfce4" \ + COPPER_XLOG="$TMP/xorg.log" COPPER_SESSIONLOG="$TMP/session.log" \ + COPPER_XPROBE="$TMP/probe" COPPER_XORGCONF="$TMP/xorg.conf" \ + XSOCKDIR="$PSOCK" WAIT_TRIES=5 sh "$SUT" 2>&1); rc=$? +pkill -f "$TMP/Xorg" 2>/dev/null; pkill -f "$PSOCK" 2>/dev/null +eq " the wait ends, with a nonzero exit" 22 "$rc" +has " says the display never opened" "never opened" "$out" + pkill -f "$TMP" 2>/dev/null rm -rf "$TMP" echo diff --git a/tests/xprobe-gate.sh b/tests/xprobe-gate.sh new file mode 100755 index 0000000..a263a89 --- /dev/null +++ b/tests/xprobe-gate.sh @@ -0,0 +1,254 @@ +#!/bin/sh +# Prove the real xprobe binary makes the liveness decision startxfce relies on, +# against a synthetic X server that speaks just enough of the wire protocol. +# +# xprobe is a static C binary; the image builds it in iso/build.sh with musl-gcc +# and stages it as /usr/bin/xprobe. This test compiles THE SOURCE with whatever +# C compiler the host has and drives the resulting binary through every socket +# state, asserting the exit-code contract described in iso/x11/probe/xprobe.c: +# +# 0 live the display answered the handshake and the geometry query +# 1 not live no socket, or the connection was refused +# 2 usage the probe was called wrongly +# 3 unknown something was reached but would not answer (timeout etc.) +# +# The synthetic server is plain python3 with no third-party modules. "reply" +# completes a real ConnectionSetup + GetGeometry round trip; "mute" accepts the +# connection and then says nothing -- the exact state that used to hang xprobe +# forever, because its reads had no timeout at all. +# +# Needs: a C compiler (gcc, cc, clang or musl-gcc) and python3. Without a +# compiler this test SKIPS and says so; it does not fail -- the Windows box +# this tree is maintained on has no gcc, and the CI image and build VM do. +# +# Run: sh tests/xprobe-gate.sh + +set -u + +REPO=$(cd "$(dirname "$0")/.." && pwd) +SRC="$REPO/iso/x11/probe/xprobe.c" +BUILD="$REPO/iso/build.sh" +TMP=$(mktemp -d /tmp/xprobe-gate.XXXXXX) || exit 1 +SOCK="$TMP/.X11-unix" +mkdir -p "$SOCK" +fails=0 + +cleanup() { + pkill -P $$ 2>/dev/null + rm -rf "$TMP" 2>/dev/null +} +trap cleanup EXIT INT TERM HUP + +ok() { printf ' ok %s\n' "$1"; } +bad() { printf ' FAIL %s\n %s\n' "$1" "${2:-}"; fails=$((fails + 1)); } +eq() { if [ "$2" = "$3" ]; then ok "$1"; else bad "$1" "wanted '$2', got '$3'"; fi; } +has() { + case "$3" in + *"$2"*) ok "$1" ;; + *) bad "$1" "expected '$2' in the output; it said: $(printf '%s' "$3" | tr '\n' '|')" ;; + esac +} + +[ -f "$SRC" ] || { echo "FAIL $SRC does not exist"; exit 1; } +[ -f "$BUILD" ] || { echo "FAIL $BUILD does not exist"; exit 1; } + +CC=${CC:-} +if [ -z "$CC" ]; then + for c in gcc cc clang musl-gcc; do + if command -v "$c" >/dev/null 2>&1; then CC=$c; break; fi + done +fi +if [ -z "$CC" ]; then + echo "=== no C compiler on this host; cannot build the real xprobe ===" + echo " SKIPPED (install gcc, or run this where the image is built)" + exit 0 +fi + +# A synthetic X server on $SOCK/X0. mode reply|mute. +fakex() { + python3 - "$SOCK/X0" "$1" >/dev/null 2>&1 <<'PY' & +import socket, struct, sys, time, os +path, mode = sys.argv[1], sys.argv[2] +try: + os.unlink(path) +except OSError: + pass +srv = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +srv.bind(path) +srv.listen(4) +while True: + conn, _ = srv.accept() + try: + req = conn.recv(12) + if len(req) < 12: + conn.close() + continue + if mode == "mute": + # Accept, read the setup request, never answer. This is the + # "socket exists, something accepts, nothing speaks X" case. + time.sleep(600) + vendor = b"fakex" + vendor_pad = (len(vendor) + 3) & ~3 + body = bytearray(32) + struct.pack_into(" [XPROBE_TIMEOUT seconds] +run() { + want=$2 + tmo=${3:-2} + DISPLAY=:0 XPROBE_SOCKET_DIR="$SOCK" XPROBE_TIMEOUT="$tmo" \ + timeout 15 "$TMP/xprobe" >"$TMP/out" 2>"$TMP/err" + rc=$? + eq " $1" "$want" "$rc" +} + +echo "=== the source compiles the way the image builds it ===" +if $CC -std=c11 -O2 -Wall -Wextra -o "$TMP/xprobe" "$SRC" 2>"$TMP/cc.err"; then + ok "compiles with $CC -std=c11 -Wall -Wextra" +else + bad "compiles with $CC -std=c11 -Wall -Wextra" "$(cat "$TMP/cc.err")" + echo "cannot continue without a binary" + exit 1 +fi +if [ -s "$TMP/cc.err" ]; then + bad "compiles without warnings" "$(cat "$TMP/cc.err")" +else + ok "compiles without warnings" +fi + +echo +echo "=== case A: no socket at all ===" +run "no socket answers 1 (not live)" 1 + +echo +echo "=== case B: a stale socket file, nothing listening ===" +python3 -c "import socket,sys +s = socket.socket(socket.AF_UNIX) +s.bind('$SOCK/X0') +s.close()" +[ -S "$SOCK/X0" ] && echo " (set up: socket file exists, nothing is listening)" +run "stale socket answers 1 (not live)" 1 + +echo +echo "=== case C: a listener that accepts but never speaks X ===" +fakex mute +fpid=$! +sleep 0.5 +start=$(date +%s) +run "mute listener answers 3 (unknown), does not hang" 3 +elapsed=$(($(date +%s) - start)) +if [ "$elapsed" -le 8 ]; then + ok " bounded: returned in ${elapsed}s instead of hanging" +else + bad "bounded" "took ${elapsed}s" +fi +kill "$fpid" 2>/dev/null; wait "$fpid" 2>/dev/null +rm -f "$SOCK/X0" + +echo +echo "=== case D: a live server answers the handshake ===" +fakex reply +fpid=$! +sleep 0.5 +run "live server answers 0 (live)" 0 +has " completed the handshake" "handshake.status=success" "$(cat "$TMP/out")" +has " read the geometry back" "getgeometry.status=ok" "$(cat "$TMP/out")" +has " reported the width" "getgeometry.width=1280" "$(cat "$TMP/out")" +has " reported the height" "getgeometry.height=800" "$(cat "$TMP/out")" +kill "$fpid" 2>/dev/null; wait "$fpid" 2>/dev/null +rm -f "$SOCK/X0" + +echo +echo "=== case E: usage errors answer 2 ===" +DISPLAY= XPROBE_SOCKET_DIR="$SOCK" XPROBE_TIMEOUT=2 "$TMP/xprobe" >/dev/null 2>&1 +eq " no DISPLAY answers 2 (usage)" 2 "$?" +DISPLAY=:0 XPROBE_SOCKET_DIR="$SOCK" XPROBE_TIMEOUT=2 "$TMP/xprobe" --bogus >/dev/null 2>&1 +eq " an unknown option answers 2 (usage)" 2 "$?" + +echo +echo "=== case F: XPROBE_TIMEOUT bounds the wait, as configured ===" +fakex mute +fpid=$! +sleep 0.5 +start=$(date +%s) +run "1s timeout returns unknown instead of hanging" 3 1 +elapsed=$(($(date +%s) - start)) +if [ "$elapsed" -ge 1 ] && [ "$elapsed" -le 6 ]; then + ok " honored: returned in ${elapsed}s (want about 1s)" +else + bad "honored timeout" "took ${elapsed}s" +fi +kill "$fpid" 2>/dev/null; wait "$fpid" 2>/dev/null +rm -f "$SOCK/X0" + +echo +echo "=== case G: iso/build.sh stages the binary built from this source ===" +# No toolchain needed: this is a static check that the build integration exists, +# so the fix cannot silently ship as "source changed but the ISO never got it". +if grep -q -- '-o "$TGT/usr/bin/xprobe"' "$BUILD"; then + ok " the build compiles it to \$TGT/usr/bin/xprobe" +else + bad " the build compiles it to \$TGT/usr/bin/xprobe" "compile line not found" +fi +if grep -q -- '[ -x "$TGT/usr/bin/xprobe" ]' "$BUILD"; then + ok " the build refuses to skip a missing /usr/bin/xprobe" +else + bad " the build refuses to skip a missing /usr/bin/xprobe" "existence check not found" +fi +if [ "$(grep -c '"$ROOT/x11/probe/xprobe.c"' "$BUILD")" -ge 2 ]; then + ok " the source feeds both the skip-stamp and the build-stamp lists" +else + bad " the source feeds both the skip-stamp and the build-stamp lists" \ + "expected >=2 mentions, found $(grep -c '"$ROOT/x11/probe/xprobe.c"' "$BUILD")" +fi + +echo +if [ "$fails" -eq 0 ]; then + echo "all xprobe checks passed" +else + echo "$fails xprobe checks FAILED" + exit 1 +fi \ No newline at end of file