From 853e2268123830d8d9a50ddea78075812c4e80b3 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Thu, 13 Aug 2026 10:15:19 +0200 Subject: [PATCH] libink/dbus: give parked and outstanding calls a deadline A call is parked until the resolver says who sent it, and an outbound call sits in a pending slot until its reply lands. Neither had a way to give up. A broker that answers GetConnectionUnixUser slowly, or not at all, leaves the caller waiting forever and keeps the slot; four of those and every later privileged call is refused with LimitsExceeded until Finit restarts. libink cannot time itself out, it has no event loop, so the deadline is the embedder's to keep. One sweep per connection covers both, and the ordering between them stays in the library rather than in each embedder: calls first, because one timing out usually resolves the park it was made for, and AccessDenied tells that caller more than a bare timeout. The sweep is armed when a resolve is deferred and stops rearming as soon as nothing is outstanding, so a system that never meets a broker never wakes up for it. Signed-off-by: Joachim Wiberg --- libink/connection.c | 40 ++++++++++++++++++++++++++++++++++ libink/dispatch.c | 52 ++++++++++++++++++++++++++++++++++++++++++--- libink/internal.h | 10 +++++++-- libink/io.c | 15 +++++++++++++ libink/link.h | 12 +++++++++++ src/dbus.c | 48 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 172 insertions(+), 5 deletions(-) diff --git a/libink/connection.c b/libink/connection.c index 755e125f..d5cff196 100644 --- a/libink/connection.c +++ b/libink/connection.c @@ -87,6 +87,7 @@ int link_connection_call(link_connection_t *conn, const char *destination, bus->pending[i].used = 1; bus->pending[i].serial = serial; + bus->pending[i].stamp = __now_ms(); bus->pending[i].cb = cb; bus->pending[i].userdata = userdata; @@ -107,6 +108,45 @@ void __bus_free(link_connection_t *conn) conn->bus = NULL; } +/* Neither half can time itself out, libink has no event loop, so the + * embedder sweeps. Calls go first: one timing out usually resolves + * the park it was made for, and being told AccessDenied says more to + * that caller than a bare timeout. */ +int link_connection_expire(link_connection_t *conn, unsigned int age_ms) +{ + struct link_bus *bus; + uint64_t now; + int i, live = 0; + + if (!conn || !conn->bus) + return 0; + + bus = conn->bus; + now = __now_ms(); + for (i = 0; i < LINK_PENDING_CAP; i++) { + link_reply_cb_t cb; + void *userdata; + + if (!bus->pending[i].used) + continue; + + if (now - bus->pending[i].stamp < age_ms) { + live++; + continue; + } + + cb = bus->pending[i].cb; + userdata = bus->pending[i].userdata; + bus->pending[i].used = 0; + + __dbg("timed out call serial %u, no reply", bus->pending[i].serial); + if (cb) + cb(conn, NULL, userdata); + } + + return live + __dispatch_expire_parked(conn, age_ms); +} + void link_connection_close(link_connection_t *conn) { size_t i; diff --git a/libink/dispatch.c b/libink/dispatch.c index c79a9b6a..5597407d 100644 --- a/libink/dispatch.c +++ b/libink/dispatch.c @@ -387,9 +387,10 @@ static struct link_parked *park(link_connection_t *conn, const uint8_t *frame, if (i == LINK_PARKED_CAP) return NULL; - bus->parked[i].tok = ++bus->next_tok; - bus->parked[i].conn = conn; - bus->parked[i].len = len; + bus->parked[i].tok = ++bus->next_tok; + bus->parked[i].conn = conn; + bus->parked[i].stamp = __now_ms(); + bus->parked[i].len = len; memcpy(bus->parked[i].buf, frame, len); *tok = bus->parked[i].tok; @@ -402,6 +403,51 @@ static void unpark(struct link_parked *p) p->conn = NULL; } +/* A resolver that answers late, or never, would otherwise hold both + * the slot and the caller forever. Returns how many are still parked; + * link_connection_expire() is what calls this. */ +int __dispatch_expire_parked(link_connection_t *conn, unsigned int age_ms) +{ + uint64_t now; + int i, live = 0; + + if (!conn->bus) + return 0; + + now = __now_ms(); + for (i = 0; i < LINK_PARKED_CAP; i++) { + struct link_parked *p = &conn->bus->parked[i]; + uint8_t buf[LINK_PARKED_MSG_MAX]; + struct link_msg msg; + size_t len; + + if (!p->tok) + continue; + + if (now - p->stamp < age_ms) { + live++; + continue; + } + + /* Free the slot before replying, as link_uid_resolved() + * does: the send path must not find it still parked. */ + len = p->len; + memcpy(buf, p->buf, len); + unpark(p); + + if (__msg_parse(buf, len, &msg) <= 0) + continue; + + __dbg("timed out %s, nobody said who the caller was", + msg.member ? msg.member : "call"); + (void)__send_error(conn, &msg, + "org.freedesktop.DBus.Error.TimedOut", + "Timed out identifying the caller"); + } + + return live; +} + void __dispatch_forget_conn(link_connection_t *conn) { struct link_bus *bus = conn->bus; diff --git a/libink/internal.h b/libink/internal.h index 6d6aaa75..e71fcf00 100644 --- a/libink/internal.h +++ b/libink/internal.h @@ -62,10 +62,12 @@ TAILQ_HEAD(link_object_list, link_object); /* An inbound method call held while we find out who sent it. The * message is copied because rxbuf is reused as soon as we return to * the read loop. `tok` is the handle the resolver answers with, and - * zero when the slot is free. */ + * zero when the slot is free. `stamp` is when it was parked, for + * link_connection_expire(). */ struct link_parked { link_authz_t tok; link_connection_t *conn; + uint64_t stamp; size_t len; uint8_t buf[LINK_PARKED_MSG_MAX]; }; @@ -86,6 +88,7 @@ struct link_bus { struct { int used; uint32_t serial; + uint64_t stamp; /* for link_connection_expire() */ link_reply_cb_t cb; void *userdata; } pending[LINK_PENDING_CAP]; @@ -177,9 +180,11 @@ void __log(const char *func, const char *fmt, ...) __attribute__((format(printf, 2, 3))); #define __dbg(fmt, ...) __log(__func__, fmt, ##__VA_ARGS__) -/* io.c — shared EINTR-resilient I/O loops. */ +/* io.c — shared EINTR-resilient I/O loops, and the clock the expiry + * sweeps measure against. */ int __io_write_all(int fd, const void *buf, size_t len); int __io_read_full(int fd, void *buf, size_t len); +uint64_t __now_ms(void); /* auth.c */ int __auth_process(link_connection_t *conn); @@ -193,6 +198,7 @@ void __bus_free(link_connection_t *conn); /* dispatch.c */ int __dispatch_message(link_connection_t *conn, const struct link_msg *m, size_t framelen); void __dispatch_forget_conn(link_connection_t *conn); +int __dispatch_expire_parked(link_connection_t *conn, unsigned int age_ms); int __send_error(link_connection_t *conn, const struct link_msg *req, const char *error_name, const char *text); int __send_method_return(link_connection_t *conn, const struct link_msg *req, diff --git a/libink/io.c b/libink/io.c index 7f95eafc..2d96045e 100644 --- a/libink/io.c +++ b/libink/io.c @@ -5,15 +5,30 @@ * read_full. On any other error they return -1 with an unknown * number of bytes already transferred. * + * Also the clock the expiry sweeps measure against: monotonic, so a + * step in wall time cannot make a call look older or younger than it + * is. + * * Copyright (c) 2026 Joachim Wiberg * SPDX-License-Identifier: MIT */ #include +#include #include #include "internal.h" +uint64_t __now_ms(void) +{ + struct timespec ts; + + if (clock_gettime(CLOCK_MONOTONIC, &ts)) + return 0; + + return (uint64_t)ts.tv_sec * 1000 + (uint64_t)(ts.tv_nsec / 1000000); +} + int __io_write_all(int fd, const void *buf, size_t len) { const char *p = buf; diff --git a/libink/link.h b/libink/link.h index f81a804b..40f6f4b9 100644 --- a/libink/link.h +++ b/libink/link.h @@ -164,6 +164,18 @@ int link_connection_call(link_connection_t *conn, const char *destination, link_reply_cb_t cb, void *userdata, const char *signature, ...); +/* Nothing in libink runs a clock, it has no event loop, so calls that + * go unanswered in either direction are the embedder's to time out. + * This drops anything held longer than `age_ms` on one connection: a + * park whose resolver never answered, which leaves its caller + * org.freedesktop.DBus.Error.TimedOut, and a call whose reply never + * came, whose callback runs once with a NULL reply exactly as a + * dropped connection would. + * + * Returns how many are still outstanding, so a sweep can stop + * rearming once nothing is left. */ +int link_connection_expire(link_connection_t *conn, unsigned int age_ms); + /* ---------- server / connection lifecycle ---------- */ /* Bind a listening socket at `path` with file mode `mode`, e.g. 0660 diff --git a/src/dbus.c b/src/dbus.c index 2a610688..8f18f1f0 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -106,6 +106,50 @@ static void peer_reap(void *arg) * stopped, so nothing touches it in the meantime. */ static struct wq reap_work = { .cb = peer_reap, .delay = 10 }; +/* + * Nothing in libink can time itself out, it has no event loop, so the + * deadline for a parked call and for a call we made on the broker is + * ours to keep. The sweep only runs while something is outstanding: + * expire_arm() starts it, and it stops rearming as soon as nothing is + * left, so a system that never talks to a broker never wakes up for + * this. + */ +#define DBUS_CALL_TIMEOUT_MS 5000 +#define DBUS_SWEEP_MS 1000 + +static void expire_sweep(void *arg); +static struct wq expire_work = { .cb = expire_sweep, .delay = DBUS_SWEEP_MS }; +static int expire_armed; + +/* Idempotent: several parks in one turn of the loop share one sweep. */ +static void expire_arm(void) +{ + if (expire_armed) + return; + if (!schedule_work(&expire_work)) + expire_armed = 1; +} + +static void expire_sweep(void *arg) +{ + struct peer *p, *tmp; + int live = 0; + + (void)arg; + expire_armed = 0; + + /* _SAFE because expiring a call runs its callback, and a + * callback that ends up writing to a peer can drop it, which + * unlinks it from this very list. */ + TAILQ_FOREACH_SAFE(p, &peers, link, tmp) { + if (!p->dead) + live += link_connection_expire(p->conn, DBUS_CALL_TIMEOUT_MS); + } + + if (live) + expire_arm(); +} + /* * A peer can be dropped from inside its own read loop: a handler emits * a signal, the write to this very peer fails, and dbus_emit_signal() @@ -1410,6 +1454,10 @@ static int sysbus_uid_resolver(link_connection_t *conn, const char *sender, return -1; } + /* Both the call and the park it belongs to now have a deadline + * to answer by, so make sure something is watching the clock. */ + expire_arm(); + return 1; /* parked; uid_reply_cb() answers */ }