From 4b274055ede3d58ceaf2ddb21bd20bcb3d26532f Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:06:32 +0200 Subject: [PATCH 01/27] libubus: fix NULL dereference on OOM in ubus_queue_msg calloc_a() can return NULL under memory pressure. The returned pointer was used immediately without a check, causing a NULL dereference crash. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libubus.c b/libubus.c index 5d22660..5526a9d 100644 --- a/libubus.c +++ b/libubus.c @@ -74,6 +74,10 @@ const char *ubus_strerror(int error) return err; } +/* + * Note: any SCM_RIGHTS fd attached to the original message is not propagated + * to the queued copy — the caller must close it (or hand it off) separately. + */ static void ubus_queue_msg(struct ubus_context *ctx, struct ubus_msghdr_buf *buf) { @@ -81,6 +85,8 @@ ubus_queue_msg(struct ubus_context *ctx, struct ubus_msghdr_buf *buf) void *data; pending = calloc_a(sizeof(*pending), &data, blob_raw_len(buf->data)); + if (!pending) + return; pending->hdr.data = data; memcpy(&pending->hdr.hdr, &buf->hdr, sizeof(buf->hdr)); From 8b5be570f13e49adf0ccc1c4fb77b0649992842d Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:06:59 +0200 Subject: [PATCH 02/27] libubus-acl: fix dangling pointers on blob_memdup failure in acl_recv_cb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit blob_memdup() can return NULL under memory pressure. The old code cleared the ACL tree and freed acl_blob first, then called blob_memdup(). On failure the tree was empty and acl_blob was NULL, but acl_add() still ran and stored pointers into the transient 'msg' buffer — dangling pointers once the caller freed that buffer. Fix by allocating the new blob first. Only clear the old state after the allocation succeeds. Also parse from the stable acl_blob copy rather than from the transient 'msg' argument. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus-acl.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libubus-acl.c b/libubus-acl.c index aab6629..8f045ad 100644 --- a/libubus-acl.c +++ b/libubus-acl.c @@ -104,6 +104,10 @@ static void acl_recv_cb(struct ubus_request *req, struct blob_attr *cur; size_t rem; + struct blob_attr *new_blob = blob_memdup(msg); + if (!new_blob) + return; + if (acl_blob) { struct acl_object *p, *q; @@ -113,9 +117,9 @@ static void acl_recv_cb(struct ubus_request *req, } free(acl_blob); } - acl_blob = blob_memdup(msg); - blobmsg_parse(acl_policy, __ACL_POLICY_MAX, tb, blobmsg_data(msg), - blobmsg_data_len(msg)); + acl_blob = new_blob; + blobmsg_parse(acl_policy, __ACL_POLICY_MAX, tb, blobmsg_data(acl_blob), + blobmsg_data_len(acl_blob)); if (!tb[ACL_POLICY_SEQ] && !tb[ACL_POLICY_ACL]) return; From 9105ea2a349a2f0de334e8b661baa76e21e47d13 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:07:13 +0200 Subject: [PATCH 03/27] ubusd_acl: fix NULL dereference on OOM in ubusd_acl_alloc_obj calloc_a() can return NULL under memory pressure. The returned pointer was dereferenced immediately on the next line without a NULL check. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_acl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ubusd_acl.c b/ubusd_acl.c index a5429cd..ac47f5b 100644 --- a/ubusd_acl.c +++ b/ubusd_acl.c @@ -316,6 +316,8 @@ ubusd_acl_alloc_obj(struct ubusd_acl_file *file, const char *obj) } o = calloc_a(sizeof(*o), &k, len + 1); + if (!o) + return NULL; o->partial = partial; o->user = file->user; o->group = file->group; From 07d7f34ac278c28817b41c8814df2cc1d303495f Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:07:41 +0200 Subject: [PATCH 04/27] ubusd_acl: handle allocation failures in ubusd_acl_init_client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related fixes for unchecked allocations on the client-init path: - strdup() for cl->group and cl->user was not checked. A failed allocation would leave NULL pointers that later crash any code comparing user or group strings (e.g. ubusd_handle_add_watch). - ubusd_acl_load_extra_gids() collapsed two unrelated failures into a single -1 return: fopen("/proc//status") failing (e.g. /proc not mounted, or the pid already gone) and malloc() of cl->extra_gid failing under memory pressure. The caller ignored the return entirely, so the malloc failure silently produced a client with no supplementary groups, which would then evaluate ACLs incorrectly. Distinguish the two: fopen failure now returns 0 (degraded but functional — no extra gids available); malloc failure returns -1 (real OOM). - Have ubusd_acl_init_client() check the extra-gid return and tear down the already-allocated user/group strings on failure, matching the existing strdup cleanup pattern. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_acl.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/ubusd_acl.c b/ubusd_acl.c index ac47f5b..e81643b 100644 --- a/ubusd_acl.c +++ b/ubusd_acl.c @@ -191,7 +191,7 @@ ubusd_acl_load_extra_gids(struct ubus_client *cl, pid_t pid) snprintf(path, sizeof(path), "/proc/%d/status", (int)pid); f = fopen(path, "r"); if (!f) - return -1; + return 0; while (fgets(line, sizeof(line), f)) { if (strncmp(line, "Groups:", 7) != 0) @@ -257,9 +257,27 @@ ubusd_acl_init_client(struct ubus_client *cl, int fd) cl->gid = cred.gid; cl->group = strdup(group->gr_name); + if (!cl->group) { + ULOG_ERR("Failed strdup() for group\n"); + return -1; + } + cl->user = strdup(pwd->pw_name); + if (!cl->user) { + ULOG_ERR("Failed strdup() for user\n"); + free(cl->group); + cl->group = NULL; + return -1; + } - ubusd_acl_load_extra_gids(cl, cred.pid); + if (ubusd_acl_load_extra_gids(cl, cred.pid)) { + ULOG_ERR("Failed to load extra gids\n"); + free(cl->user); + cl->user = NULL; + free(cl->group); + cl->group = NULL; + return -1; + } return 0; } From 497321a5ea90093fbf05d786a120afd3f8bdb6fb Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:08:04 +0200 Subject: [PATCH 05/27] ubusd_acl: fix NULL dereference on OOM in ubusd_acl_init ubusd_create_object_internal() can return NULL if ubus_alloc_id() fails (e.g. when reading from /dev/urandom fails). The recv_msg assignment was performed unconditionally, causing a NULL dereference. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_acl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ubusd_acl.c b/ubusd_acl.c index e81643b..ec1aacb 100644 --- a/ubusd_acl.c +++ b/ubusd_acl.c @@ -686,5 +686,6 @@ void ubusd_acl_init(void) { ubus_init_string_tree(&ubusd_acls, true); acl_obj = ubusd_create_object_internal(NULL, UBUS_SYSTEM_OBJECT_ACL); - acl_obj->recv_msg = ubusd_acl_recv; + if (acl_obj) + acl_obj->recv_msg = ubusd_acl_recv; } From f66d52ba983fc588f25d56c419a9101fe24a0e53 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:08:29 +0200 Subject: [PATCH 06/27] ubusd_event: fix OOM handling in ubusd_send_event_msg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fill_cb() calls ubus_msg_new(), which can return NULL under memory pressure. The returned pointer was dereferenced unconditionally on the next line to set hdr.type and hdr.peer, causing a NULL dereference crash. ubusd_send_event_msg() also returned void, so catching that NULL still left the function returning silently while the caller in ubusd_send_event() kept iterating the pattern tree. Every remaining iteration called fill_cb() again (almost certainly hitting the same OOM) and ubusd_send_event() unconditionally returned 0 — the originating client received UBUS_STATUS_OK even though no subscriber had been delivered the event. Check the fill_cb() result for NULL, convert ubusd_send_event_msg() to return int and surface the OOM as UBUS_STATUS_NO_DATA (the status the rest of this file already uses for allocation failures, e.g. ubusd_alloc_event_pattern()). ubusd_send_event() now breaks out of the loop on the first error and propagates the status, which flows back through ubusd_forward_event() to the originating client so it knows the send did not complete. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_event.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/ubusd_event.c b/ubusd_event.c index 3036969..419c018 100644 --- a/ubusd_event.c +++ b/ubusd_event.c @@ -111,24 +111,26 @@ static int ubusd_alloc_event_pattern(struct ubus_client *cl, struct blob_attr *m return 0; } -static void ubusd_send_event_msg(struct ubus_msg_buf **ub, struct ubus_client *cl, - struct ubus_object *obj, const char *id, - event_fill_cb fill_cb, void *cb_priv) +static int ubusd_send_event_msg(struct ubus_msg_buf **ub, struct ubus_client *cl, + struct ubus_object *obj, const char *id, + event_fill_cb fill_cb, void *cb_priv) { uint32_t *objid_ptr; /* do not loop back events */ if (obj->client == cl) - return; + return 0; /* do not send duplicate events */ if (obj->event_seen == obj_event_seq) - return; + return 0; obj->event_seen = obj_event_seq; if (!*ub) { *ub = fill_cb(cb_priv, id); + if (!*ub) + return UBUS_STATUS_UNKNOWN_ERROR; (*ub)->hdr.type = UBUS_MSG_INVOKE; (*ub)->hdr.peer = 0; } @@ -138,6 +140,7 @@ static void ubusd_send_event_msg(struct ubus_msg_buf **ub, struct ubus_client *c (*ub)->hdr.seq = ++event_seq; ubus_msg_send(obj->client, *ub); + return 0; } int ubusd_send_event(struct ubus_client *cl, const char *id, @@ -146,6 +149,7 @@ int ubusd_send_event(struct ubus_client *cl, const char *id, struct ubus_msg_buf *ub = NULL; struct event_source *ev; int match_len = 0; + int ret = 0; if (ubusd_acl_check(cl, id, NULL, UBUS_ACL_SEND)) return UBUS_STATUS_PERMISSION_DENIED; @@ -176,13 +180,15 @@ int ubusd_send_event(struct ubus_client *cl, const char *id, continue; } - ubusd_send_event_msg(&ub, cl, ev->obj, id, fill_cb, cb_priv); + ret = ubusd_send_event_msg(&ub, cl, ev->obj, id, fill_cb, cb_priv); + if (ret) + break; } if (ub) ubus_msg_free(ub); - return 0; + return ret; } enum { From 11ea1b3bdbea5ec5bd27fefff661a5f1b970d384 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:09:26 +0200 Subject: [PATCH 07/27] ubusd_main: fix async-signal-unsafe SIGHUP handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SIGHUP handler called ubusd_acl_load() directly, which in turn calls malloc, fopen, glob, syslog, and avl_insert — none of which are async-signal-safe. Invoking them from a signal handler that interrupted any of those same functions in the main loop can cause heap corruption, deadlock, or a crash. Replace with a self-pipe: the signal handler writes a single byte (write() is async-signal-safe) to a pipe, and a uloop fd watcher drains the pipe and calls ubusd_acl_load() safely from the event loop context. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_main.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/ubusd_main.c b/ubusd_main.c index 0e4c6d8..19c2ad1 100644 --- a/ubusd_main.c +++ b/ubusd_main.c @@ -4,12 +4,15 @@ * SPDX-License-Identifier: LGPL-2.1-only */ +#define _GNU_SOURCE + #include #include #include #ifdef FreeBSD #include #endif +#include #include #include @@ -245,11 +248,29 @@ static int usage(const char *progname) return 1; } +static int sighup_pipe[2] = { -1, -1 }; + static void sighup_handler(int sig) { + int saved_errno = errno; + char c = 0; + ssize_t r = write(sighup_pipe[1], &c, sizeof(c)); + (void)r; + errno = saved_errno; +} + +static void sighup_fd_cb(struct uloop_fd *ufd, unsigned int events) +{ + char buf[16]; + while (read(ufd->fd, buf, sizeof(buf)) > 0) + ; ubusd_acl_load(); } +static struct uloop_fd sighup_ufd = { + .cb = sighup_fd_cb, +}; + static void mkdir_sockdir() { char *ubus_sock_dir, *tmp; @@ -272,12 +293,32 @@ int main(int argc, char **argv) int ch; signal(SIGPIPE, SIG_IGN); - signal(SIGHUP, sighup_handler); + +#ifdef __linux__ + ret = pipe2(sighup_pipe, O_NONBLOCK | O_CLOEXEC); +#else + ret = pipe(sighup_pipe); + if (ret == 0) { + fcntl(sighup_pipe[0], F_SETFL, O_NONBLOCK); + fcntl(sighup_pipe[0], F_SETFD, FD_CLOEXEC); + fcntl(sighup_pipe[1], F_SETFL, O_NONBLOCK); + fcntl(sighup_pipe[1], F_SETFD, FD_CLOEXEC); + } +#endif + if (ret == 0) + signal(SIGHUP, sighup_handler); + else + signal(SIGHUP, SIG_IGN); ulog_open(ULOG_KMSG | ULOG_SYSLOG, LOG_DAEMON, "ubusd"); openlog("ubusd", LOG_PID, LOG_DAEMON); uloop_init(); + if (sighup_pipe[0] >= 0) { + sighup_ufd.fd = sighup_pipe[0]; + uloop_fd_add(&sighup_ufd, ULOOP_READ); + } + while ((ch = getopt(argc, argv, "A:s:")) != -1) { switch (ch) { case 's': From 0c095592ccb79053dd02a57bf006dde2bd26ad28 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:09:52 +0200 Subject: [PATCH 08/27] ubusd_proto: fix resource leaks and ID tree corruption in ubusd_proto_new_client Three bugs in the error cleanup paths: 1. When ubus_alloc_id() failed, 'goto free' skipped ubusd_acl_free_client(), leaking the user/group strings and extra_gid array. 2. When ubusd_proto_init_retmsg() failed, 'goto free' skipped both ubus_free_id() and ubusd_acl_free_client(). The freed 'cl' struct remained in the client ID tree, causing use-after-free on any subsequent ID lookup. 3. When ubusd_send_hello() failed, 'goto delete' called ubus_free_id() but did not free cl->retmsg or cl->b, leaking those allocations. Fix by restructuring the error labels so each path frees exactly the resources that were successfully allocated. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_proto.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ubusd_proto.c b/ubusd_proto.c index 31263dc..a25a2ce 100644 --- a/ubusd_proto.c +++ b/ubusd_proto.c @@ -583,10 +583,10 @@ struct ubus_client *ubusd_proto_new_client(int fd, uloop_fd_handler cb) cl->pending_msg_fd = -1; if (!ubus_alloc_id(&clients, &cl->id, 0)) - goto free; + goto acl_free; if (ubusd_proto_init_retmsg(cl)) - goto free; + goto delete_no_retmsg; if (!ubusd_send_hello(cl)) goto delete; @@ -594,7 +594,12 @@ struct ubus_client *ubusd_proto_new_client(int fd, uloop_fd_handler cb) return cl; delete: + ubus_msg_free(cl->retmsg); +delete_no_retmsg: + blob_buf_free(&cl->b); ubus_free_id(&clients, &cl->id); +acl_free: + ubusd_acl_free_client(cl); free: free(cl); return NULL; From f61695e6e12a622f6d6408c444f162ea6f13fdac Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:10:19 +0200 Subject: [PATCH 09/27] ubusd_proto: fix NULL dereference for user/group in ubusd_handle_add_watch cl->user, cl->group, target->client->user, and target->client->group can all be NULL if the earlier strdup() calls in ubusd_acl_init_client() fail under memory pressure. Passing a NULL pointer to strcmp() is undefined behaviour and causes a crash. Guard each comparison with explicit NULL checks and treat NULL as non-matching (deny access) to preserve the original security intent. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_proto.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ubusd_proto.c b/ubusd_proto.c index a25a2ce..8950d57 100644 --- a/ubusd_proto.c +++ b/ubusd_proto.c @@ -461,7 +461,11 @@ static int ubusd_handle_add_watch(struct ubus_client *cl, struct ubus_msg_buf *u return UBUS_STATUS_INVALID_ARGUMENT; if (!target->path.key) { - if (strcmp(target->client->user, cl->user) && strcmp(target->client->group, cl->group)) + bool same_user = target->client->user && cl->user && + !strcmp(target->client->user, cl->user); + bool same_group = target->client->group && cl->group && + !strcmp(target->client->group, cl->group); + if (!same_user && !same_group) return UBUS_STATUS_NOT_FOUND; } else if (ubusd_acl_check(cl, target->path.key, NULL, UBUS_ACL_SUBSCRIBE)) { return UBUS_STATUS_NOT_FOUND; From 7ecacfadd9bca05fae69690207c11ab894489e3f Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 10:28:38 +0200 Subject: [PATCH 10/27] ubusd_proto: fix NULL dereference on OOM in ubusd_proto_init_retmsg blob_buf_init() can return -ENOMEM if the initial buffer allocation fails. The return value was ignored, leaving cl->b.head as NULL. The subsequent blob_raw_len(b->head) then dereferences that NULL pointer, causing a crash. Check the return value and propagate the error early. The caller's goto delete_no_retmsg path already calls blob_buf_free(&cl->b), which is safe because a failed blob_buf_init leaves cl->b.buf as NULL (free(NULL) is a no-op). Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_proto.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ubusd_proto.c b/ubusd_proto.c index 8950d57..92fbd55 100644 --- a/ubusd_proto.c +++ b/ubusd_proto.c @@ -556,7 +556,8 @@ static int ubusd_proto_init_retmsg(struct ubus_client *cl) { struct blob_buf *b = &cl->b; - blob_buf_init(&cl->b, 0); + if (blob_buf_init(&cl->b, 0)) + return -1; blob_put_int32(&cl->b, UBUS_ATTR_STATUS, 0); /* we make the 'retmsg' buffer shared with the blob_buf b, to reduce mem duplication */ From 3ab9d77595457f56ed1f26350ce15d308e50f09f Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:10:57 +0200 Subject: [PATCH 11/27] lua: fix inverted argument check in ubus_lua_add The guard 'if (lua_istable(L, 1))' was backwards: it raised an error when argument 1 IS a table, but argument 1 is always the ubus_context userdata (never a table). The intent was to reject calls where argument 2 is NOT a table. As a result, any call to ubus.add(c, non_table) silently proceeded to lua_next() on a non-table value, causing a crash, while a valid call with a proper table as argument 2 was never incorrectly rejected only because arg 1 never matches lua_istable. Fix by checking !lua_istable(L, 2). Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- lua/ubus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/ubus.c b/lua/ubus.c index 07b816d..47dd837 100644 --- a/lua/ubus.c +++ b/lua/ubus.c @@ -559,7 +559,7 @@ static int ubus_lua_add(lua_State *L) struct ubus_lua_connection *c = luaL_checkudata(L, 1, METANAME); /* verify top level object */ - if (lua_istable(L, 1)) { + if (!lua_istable(L, 2)) { lua_pushstring(L, "you need to pass a table"); return lua_error(L); } From 43051ca73aec19c53d20e17e0e37a8ebd7d52d56 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:11:26 +0200 Subject: [PATCH 12/27] lua: fix unchecked calloc and memory leak in ubus_lua_load_object Two bugs in the method array allocation: 1. calloc(mlen, sizeof(struct ubus_method)) was not checked for NULL. On OOM the returned NULL pointer was stored in obj->o.methods and later dereferenced as m[midx], causing a crash. 2. calloc(0, N) has implementation-defined behaviour (may return NULL or a unique pointer). Treat mlen == 0 explicitly by setting m = NULL and skipping the allocation. 3. When the subsequent calloc for obj->o.type failed, the already- allocated method array 'm' was not freed, causing a memory leak. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- lua/ubus.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lua/ubus.c b/lua/ubus.c index 47dd837..7611fb0 100644 --- a/lua/ubus.c +++ b/lua/ubus.c @@ -491,7 +491,7 @@ static struct ubus_object* ubus_lua_load_object(lua_State *L) { struct ubus_lua_object *obj = NULL; int mlen = lua_gettablelen(L, -1); - struct ubus_method *m; + struct ubus_method *m = NULL; int midx = 0; /* setup object pointers */ @@ -502,12 +502,19 @@ static struct ubus_object* ubus_lua_load_object(lua_State *L) obj->o.name = lua_tostring(L, -2); /* setup method pointers */ - m = calloc(mlen, sizeof(struct ubus_method)); + if (mlen > 0) { + m = calloc(mlen, sizeof(struct ubus_method)); + if (!m) { + free(obj); + return NULL; + } + } obj->o.methods = m; /* setup type pointers */ obj->o.type = calloc(1, sizeof(struct ubus_object_type)); if (!obj->o.type) { + free(m); free(obj); return NULL; } From 4ca0b141e9a72a8a87feff3aa6f3ae57a9827177 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Mon, 13 Apr 2026 00:20:23 +0200 Subject: [PATCH 13/27] ubusd_id: use getrandom(2) unconditionally on Linux getrandom() avoids opening a file descriptor, works before /dev/urandom is accessible (e.g. early boot inside a container), and blocks until the kernel entropy pool is initialised, making it safer than reading from /dev/urandom directly. getrandom() is available since glibc 2.25 and musl 1.1.20. On Linux call it from directly. Non-Linux platforms continue to use the /dev/urandom fd path unchanged. Co-Authored-By: Claude Sonnet 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_id.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/ubusd_id.c b/ubusd_id.c index 8d9fede..e48410e 100644 --- a/ubusd_id.c +++ b/ubusd_id.c @@ -15,12 +15,33 @@ #include #include #include +#ifdef __linux__ +#include +#endif #include #include "ubusmsg.h" #include "ubusd_id.h" +#ifndef __linux__ static int random_fd = -1; +#endif + +static ssize_t read_random(void *buf, size_t len) +{ +#ifdef __linux__ + return getrandom(buf, len, 0); +#else + if (random_fd < 0) { + random_fd = open("/dev/urandom", O_RDONLY); + if (random_fd < 0) { + perror("open /dev/urandom"); + return -1; + } + } + return read(random_fd, buf, len); +#endif +} static int ubus_cmp_id(const void *k1, const void *k2, void *ptr) { @@ -39,14 +60,6 @@ void ubus_init_string_tree(struct avl_tree *tree, bool dup) void ubus_init_id_tree(struct avl_tree *tree) { - if (random_fd < 0) { - random_fd = open("/dev/urandom", O_RDONLY); - if (random_fd < 0) { - perror("open"); - exit(1); - } - } - avl_init(tree, ubus_cmp_id, false, NULL); } @@ -59,7 +72,7 @@ bool ubus_alloc_id(struct avl_tree *tree, struct ubus_id *id, uint32_t val) } do { - if (read(random_fd, &id->id, sizeof(id->id)) != sizeof(id->id)) + if (read_random(&id->id, sizeof(id->id)) != sizeof(id->id)) return false; if (id->id < UBUS_SYSTEM_OBJECT_MAX) From 7e4356da8abea2351643962376bc91bbc9e8ad5c Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 16 Apr 2026 22:22:26 +0200 Subject: [PATCH 14/27] ubusd_monitor: fix NULL dereference on OOM in ubusd_monitor_message ubus_msg_new() can return NULL on allocation failure, but the subsequent assignment to ub->hdr.type was unconditional, causing a NULL pointer dereference. Bail out early when allocation fails. Co-Authored-By: Claude Opus 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_monitor.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ubusd_monitor.c b/ubusd_monitor.c index bba741e..cad6b37 100644 --- a/ubusd_monitor.c +++ b/ubusd_monitor.c @@ -91,6 +91,8 @@ ubusd_monitor_message(struct ubus_client *cl, struct ubus_msg_buf *ub, bool send blob_put(&mb, UBUS_MONITOR_DATA, blob_data(ub->data), blob_len(ub->data)); ub = ubus_msg_new(mb.head, blob_raw_len(mb.head), true); + if (!ub) + return; ub->hdr.type = UBUS_MSG_MONITOR; list_for_each_entry(m, &monitors, list) { From 5849870f2251e7ca67804510446a4d6ca9e9dba2 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 16 Apr 2026 22:23:04 +0200 Subject: [PATCH 15/27] libubus-req: fix file descriptor leaks in ubus_process_req_msg Incoming messages can carry a file descriptor via SCM_RIGHTS. Several paths through ubus_process_req_msg() leaked that fd: - UBUS_MSG_STATUS with no matching request: break without closing. - UBUS_MSG_DATA: fd was never handled at all. Consolidate cleanup by closing fd at the end of the function unless it was handed off to req->fd_cb. Co-Authored-By: Claude Opus 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus-req.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libubus-req.c b/libubus-req.c index 4b718dc..8e99a62 100644 --- a/libubus-req.c +++ b/libubus-req.c @@ -487,10 +487,10 @@ void __hidden ubus_process_req_msg(struct ubus_context *ctx, struct ubus_msghdr_ break; if (fd >= 0) { - if (req->fd_cb) + if (req->fd_cb) { req->fd_cb(req, fd); - else - close(fd); + fd = -1; + } } if (id >= 0) @@ -505,6 +505,9 @@ void __hidden ubus_process_req_msg(struct ubus_context *ctx, struct ubus_msghdr_ ubus_process_req_data(req, buf); break; } + + if (fd >= 0) + close(fd); } int __ubus_monitor(struct ubus_context *ctx, const char *type) From f29767f90af112040cdf8a1ee402e5d3d48115b3 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 16 Apr 2026 22:24:13 +0200 Subject: [PATCH 16/27] libubus: fix file descriptor leaks in ubus_process_msg ubus_process_msg() receives a file descriptor along with each message but leaked it on several paths: - UBUS_MSG_NOTIFY / UBUS_MSG_UNSUBSCRIBE on a channel context. - UBUS_MSG_INVOKE queued via ubus_queue_msg() when stack_depth > 0 (ubus_queue_msg() does not propagate the fd). - UBUS_MSG_MONITOR in all paths; neither the channel-context check nor the monitor_cb invocation take ownership of the fd. - Any message type not matched by the switch. Close the fd at the end of the function for all paths that do not hand it off to a lower-level handler. Co-Authored-By: Claude Opus 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libubus.c b/libubus.c index 5526a9d..555930d 100644 --- a/libubus.c +++ b/libubus.c @@ -103,7 +103,7 @@ ubus_process_msg(struct ubus_context *ctx, struct ubus_msghdr_buf *buf, int fd) case UBUS_MSG_STATUS: case UBUS_MSG_DATA: ubus_process_req_msg(ctx, buf, fd); - break; + return; case UBUS_MSG_UNSUBSCRIBE: case UBUS_MSG_NOTIFY: @@ -119,7 +119,7 @@ ubus_process_msg(struct ubus_context *ctx, struct ubus_msghdr_buf *buf, int fd) ctx->stack_depth++; ubus_process_obj_msg(ctx, buf, fd); ctx->stack_depth--; - break; + return; case UBUS_MSG_MONITOR: if (ubus_context_is_channel(ctx)) break; @@ -128,6 +128,9 @@ ubus_process_msg(struct ubus_context *ctx, struct ubus_msghdr_buf *buf, int fd) ctx->monitor_cb(ctx, buf->hdr.seq, buf->data); break; } + + if (fd >= 0) + close(fd); } static void ubus_process_pending_msg(struct uloop_timeout *timeout) From b099d050b59d83f6f0962ac0cf8e5afd80678049 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 16 Apr 2026 22:24:56 +0200 Subject: [PATCH 17/27] libubus: make ubus_shutdown idempotent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ubus_shutdown() left ctx->sock.fd and ctx->msgbuf.data dangling after releasing them. The header documents shutdown as paired one-to-one with ubus_connect_ctx(), but the function is exposed and a caller that violates the contract — most realistically by invoking ubus_shutdown() explicitly and then ubus_free() (which calls shutdown a second time) — would close an unrelated descriptor the kernel may have recycled in the interim and double-free the msgbuf. Guard the close with a validity check and reset both fields after release so subsequent calls become no-ops. This also brings the tear- down path in line with ubus_reconnect()'s error handling, which already sets sock.fd to -1 after closing. Co-Authored-By: Claude Opus 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libubus.c b/libubus.c index 555930d..f5a50e9 100644 --- a/libubus.c +++ b/libubus.c @@ -447,9 +447,13 @@ void ubus_shutdown(struct ubus_context *ctx) if (!ctx) return; uloop_fd_delete(&ctx->sock); - close(ctx->sock.fd); + if (ctx->sock.fd >= 0) { + close(ctx->sock.fd); + ctx->sock.fd = -1; + } uloop_timeout_cancel(&ctx->pending_timer); free(ctx->msgbuf.data); + ctx->msgbuf.data = NULL; } void ubus_free(struct ubus_context *ctx) From a564b8dcb395738be3d24750ea41f1dc3a42809d Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 16 Apr 2026 22:27:38 +0200 Subject: [PATCH 18/27] ubusd_main: check strdup return value in mkdir_sockdir strdup() can fail and return NULL; the subsequent strrchr() call then dereferences a NULL pointer. The impact is limited to startup with a constant path, but handle the allocation failure gracefully. Co-Authored-By: Claude Opus 4.6 Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_main.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/ubusd_main.c b/ubusd_main.c index 19c2ad1..14d3be2 100644 --- a/ubusd_main.c +++ b/ubusd_main.c @@ -271,17 +271,32 @@ static struct uloop_fd sighup_ufd = { .cb = sighup_fd_cb, }; -static void mkdir_sockdir() +static int mkdir_sockdir() { char *ubus_sock_dir, *tmp; + int ret = 0; ubus_sock_dir = strdup(UBUS_UNIX_SOCKET); + if (!ubus_sock_dir) + return -1; + tmp = strrchr(ubus_sock_dir, '/'); if (tmp) { *tmp = '\0'; - mkdir(ubus_sock_dir, 0755); + ret = mkdir(ubus_sock_dir, 0755); + if (ret && errno == EEXIST) { + struct stat st; + + if (stat(ubus_sock_dir, &st) == 0 && S_ISDIR(st.st_mode)) + ret = 0; + else + fprintf(stderr, "%s exists but is not a directory\n", + ubus_sock_dir); + } } + free(ubus_sock_dir); + return ret; } #include @@ -332,7 +347,9 @@ int main(int argc, char **argv) } } - mkdir_sockdir(); + ret = mkdir_sockdir(); + if (ret) + goto out; unlink(ubus_socket); umask(0111); server_fd.fd = usock(USOCK_UNIX | USOCK_SERVER | USOCK_NONBLOCK, ubus_socket, NULL); From 239edcbaaac85fe2dd80c985823a826ad18adabc Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Sun, 3 May 2026 22:43:02 +0200 Subject: [PATCH 19/27] ubusd_id: fix continue in do-while skipping random ID retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In ubus_alloc_id() the `continue` statement was intended to discard random IDs that fall in the reserved system-object range (0..UBUS_SYSTEM_OBJECT_MAX-1) and retry the read. In a do-while loop, however, `continue` jumps to the loop-continuation expression, not back to the start of the body. The expression then calls avl_insert() with the still-reserved ID, and on success the loop exits — successfully assigning a reserved ID to a client, object, or object type. This breaks invariants relied upon elsewhere, e.g. ubusd_alloc_event_pattern() rejects any object with id < UBUS_SYSTEM_OBJECT_MAX as a permission error, making event registration fail for any client object that happens to draw a low random ID. Wrap the read in an inner do-while so a low ID actually triggers a re-read before insertion is attempted. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_id.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ubusd_id.c b/ubusd_id.c index e48410e..209cbb7 100644 --- a/ubusd_id.c +++ b/ubusd_id.c @@ -72,11 +72,10 @@ bool ubus_alloc_id(struct avl_tree *tree, struct ubus_id *id, uint32_t val) } do { - if (read_random(&id->id, sizeof(id->id)) != sizeof(id->id)) - return false; - - if (id->id < UBUS_SYSTEM_OBJECT_MAX) - continue; + do { + if (read_random(&id->id, sizeof(id->id)) != sizeof(id->id)) + return false; + } while (id->id < UBUS_SYSTEM_OBJECT_MAX); } while (avl_insert(tree, &id->avl) != 0); return true; From 09d2df45bf3862fc259d5b1e1527c8777e2e5d23 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Sun, 3 May 2026 22:44:02 +0200 Subject: [PATCH 20/27] ubusd: fix NULL dereference on OOM in ubus_msg_enqueue ubus_msg_ref() can return NULL when the source message uses an external buffer and the ubus_msg_new() allocation fails. ubus_msg_enqueue() stored the result without checking and queued the list entry anyway. Later, when the queue entry was freed via ubus_msg_list_free() -> ubus_msg_free(NULL), the NULL message pointer was dereferenced. Drop the queue entry and return early if the reference cannot be taken, so we never link a list entry with a NULL message. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ubusd.c b/ubusd.c index 13557d1..c04ad82 100644 --- a/ubusd.c +++ b/ubusd.c @@ -153,6 +153,10 @@ static void ubus_msg_enqueue(struct ubus_client *cl, struct ubus_msg_buf *ub) INIT_LIST_HEAD(&ubl->list); ubl->msg = ubus_msg_ref(ub); + if (!ubl->msg) { + free(ubl); + return; + } list_add_tail(&ubl->list, &cl->tx_queue); cl->txq_len += ub->len + sizeof(ub->hdr); From bcc45ca981fd61e11934776bcee18f628156a7f7 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Sun, 3 May 2026 22:58:55 +0200 Subject: [PATCH 21/27] libubus: actually set FD_CLOEXEC on the ubus socket fcntl(fd, F_SETFL, ... | O_CLOEXEC) is a no-op for the close-on-exec bit. F_SETFL only manipulates file *status* flags (O_APPEND, O_ASYNC, O_NONBLOCK, ...), and silently masks out anything else; close-on-exec is a file *descriptor* flag that has to be set with F_SETFD/FD_CLOEXEC. Both ubus_reconnect() and ubus_channel_connect() relied on the broken form and therefore left the ubus socket inheritable across exec(2). Any client that fork+exec'd a child (a common pattern in OpenWrt services) would leak the ubus connection into the new process. Set FD_CLOEXEC explicitly via F_SETFD in addition to the F_SETFL call that toggles O_NONBLOCK. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus-io.c | 8 ++++++-- libubus.c | 10 ++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/libubus-io.c b/libubus-io.c index beea3bf..afd24b2 100644 --- a/libubus-io.c +++ b/libubus-io.c @@ -443,9 +443,13 @@ int ubus_reconnect(struct ubus_context *ctx, const char *path) if (ctx->local_id <= UBUS_CLIENT_ID_CHANNEL) goto out_free; - ret = UBUS_STATUS_OK; - fcntl(ctx->sock.fd, F_SETFL, fcntl(ctx->sock.fd, F_GETFL) | O_NONBLOCK | O_CLOEXEC); + int flags = fcntl(ctx->sock.fd, F_GETFL); + if (flags < 0 || + fcntl(ctx->sock.fd, F_SETFL, flags | O_NONBLOCK) < 0 || + fcntl(ctx->sock.fd, F_SETFD, FD_CLOEXEC) < 0) + goto out_free; + ret = UBUS_STATUS_OK; ubus_refresh_state(ctx); out_free: diff --git a/libubus.c b/libubus.c index f5a50e9..5f1c28c 100644 --- a/libubus.c +++ b/libubus.c @@ -356,14 +356,20 @@ int ubus_channel_connect(struct ubus_context *ctx, int fd, close(ctx->sock.fd); } + int flags = fcntl(fd, F_GETFL); + if (flags < 0 || + fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0 || + fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) { + ctx->sock.fd = -1; + return -1; + } + ctx->sock.eof = false; ctx->sock.error = false; ctx->sock.fd = fd; ctx->local_id = UBUS_CLIENT_ID_CHANNEL; ctx->request_handler = handler; - fcntl(ctx->sock.fd, F_SETFL, fcntl(ctx->sock.fd, F_GETFL) | O_NONBLOCK | O_CLOEXEC); - return 0; } From 8188f5ce8564e7f4114e2db2dded1e6361957b2a Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Sun, 3 May 2026 23:00:25 +0200 Subject: [PATCH 22/27] libubus-io: close recv_fd captured before get_next_msg failure recv_retry() captures any SCM_RIGHTS fd into the caller's *recv_fd as soon as the first recvmsg succeeds, then continues looping to drain the rest of the iov. If a subsequent recvmsg in the same call returns 0 (EOF) or fails fatally, recv_retry returns -1 with the captured fd already written to the caller's variable. The same hand-off survives errors in the body read or in the validate/alloc path of get_next_msg(). ubus_handle_data() simply broke out of its loop in those cases, dropping the fd on the floor. With long-running clients on a flaky link this is a slow descriptor leak that eventually exhausts the process's fd table. Reset recv_fd to -1 once it has been handed off to ubus_process_msg(), and close it at loop exit if anything is still parked there. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus-io.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libubus-io.c b/libubus-io.c index afd24b2..04b6eb5 100644 --- a/libubus-io.c +++ b/libubus-io.c @@ -321,10 +321,19 @@ void __hidden ubus_handle_data(struct uloop_fd *u, unsigned int events) if (!get_next_msg(ctx, &recv_fd)) break; ubus_process_msg(ctx, &ctx->msgbuf, recv_fd); + recv_fd = -1; if (uloop_cancelling() || ctx->cancel_poll) break; } + /* + * If get_next_msg() captured an fd via the header recvmsg but then + * failed (header validate, body recv, alloc), the fd is sitting in + * recv_fd unowned by anyone — close it instead of leaking. + */ + if (recv_fd >= 0) + close(recv_fd); + if (!ctx->stack_depth) ctx->pending_timer.cb(&ctx->pending_timer); From 7a068bac5a9b877805f94797915d8d14fd434fb0 Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Sun, 3 May 2026 23:01:43 +0200 Subject: [PATCH 23/27] libubus-io: byte-swap peer in HELLO when storing as local_id Every other receive path converts the wire-format ubus_msghdr fields to host byte order before use (see get_next_msg() and ubusd_main client_cb). ubus_reconnect() forgot to do this for the HELLO peer field and stored the raw big-endian value into ctx->local_id. On little-endian hosts this means local_id holds a byte-swapped client id forever: - The defensive `if (ctx->local_id <= UBUS_CLIENT_ID_CHANNEL)` check no longer rejects an id of 1, since 0x01000000 > 1. The check was intended to catch a server handing out the channel id (1) to a regular client. - ubus_context_is_channel() can never match an actual non-channel local_id by accident, but the value is still semantically wrong and breaks any future code that treats local_id as a real id. Apply be32_to_cpu() to match the other receive paths. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libubus-io.c b/libubus-io.c index 04b6eb5..262870e 100644 --- a/libubus-io.c +++ b/libubus-io.c @@ -448,7 +448,7 @@ int ubus_reconnect(struct ubus_context *ctx, const char *path) if (read(ctx->sock.fd, blob_data(buf), blob_len(buf)) != (ssize_t) blob_len(buf)) goto out_free; - ctx->local_id = hdr.hdr.peer; + ctx->local_id = be32_to_cpu(hdr.hdr.peer); if (ctx->local_id <= UBUS_CLIENT_ID_CHANNEL) goto out_free; From 747013f6ea05a168d000531553f8de90f8b87b7f Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Sun, 3 May 2026 23:02:05 +0200 Subject: [PATCH 24/27] libubus-io: reset sock.fd to -1 after close on ubus_reconnect error path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If ubus_reconnect() fails after the new socket has been opened (HELLO read failure, header validation, OOM, body read failure, ID validation), the out_close: handler closes ctx->sock.fd but leaves the field pointing at the now-closed descriptor. A subsequent ubus_shutdown() / ubus_free() then calls close() on that stale value, and a second reconnect attempt will pass the entry guard at the top of ubus_reconnect() and close it again. In both cases the kernel may have recycled the slot for an unrelated descriptor in the meantime, causing us to close someone else's fd — the same class of bug fixed in ubus_shutdown() previously. Set ctx->sock.fd = -1 alongside the close so the field is a faithful indicator of socket ownership. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- libubus-io.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libubus-io.c b/libubus-io.c index 262870e..4d5f2e1 100644 --- a/libubus-io.c +++ b/libubus-io.c @@ -464,8 +464,10 @@ int ubus_reconnect(struct ubus_context *ctx, const char *path) out_free: free(buf); out_close: - if (ret) + if (ret) { close(ctx->sock.fd); + ctx->sock.fd = -1; + } return ret; } From 020a64b9b1693295a3fde34147aef48bf47c76ae Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 21 May 2026 23:21:55 +0200 Subject: [PATCH 25/27] ubusd_acl: use size_t for strlen result in ubusd_acl_alloc_obj len held the result of strlen() in an int. It is later passed as len + 1 to calloc_a(), whose variadic length arguments are read via va_arg as size_t in __calloc_a(). Passing an int where size_t is expected is undefined behaviour and can yield a wrong allocation size on LP64 platforms. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_acl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ubusd_acl.c b/ubusd_acl.c index ec1aacb..3cef2ee 100644 --- a/ubusd_acl.c +++ b/ubusd_acl.c @@ -321,7 +321,7 @@ static struct ubusd_acl_obj* ubusd_acl_alloc_obj(struct ubusd_acl_file *file, const char *obj) { struct ubusd_acl_obj *o; - int len = strlen(obj); + size_t len = strlen(obj); char *k; bool partial = false; From f92ffd289dcc07b20d0fcf34e10e5c685240e5bd Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 21 May 2026 23:45:59 +0200 Subject: [PATCH 26/27] ubusd: use size_t for string and blob length variables strlen() and blob_raw_len() return size_t; storing their result in an int relies on an implicit narrowing conversion. Use size_t for the affected length variables in ubusd_alloc_event_pattern(), ubus_create_obj_method() and __ubusd_handle_lookup(). ubusd_alloc_event_pattern() checked the length with "<= 0", which is only ever true for 0 once the variable is unsigned; replace it with "!len". Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_event.c | 4 ++-- ubusd_obj.c | 2 +- ubusd_proto.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ubusd_event.c b/ubusd_event.c index 419c018..47d89ca 100644 --- a/ubusd_event.c +++ b/ubusd_event.c @@ -61,7 +61,7 @@ static int ubusd_alloc_event_pattern(struct ubus_client *cl, struct blob_attr *m char *pattern, *name; uint32_t id; bool partial = false; - int len; + size_t len; if (!msg) return UBUS_STATUS_INVALID_ARGUMENT; @@ -84,7 +84,7 @@ static int ubusd_alloc_event_pattern(struct ubus_client *cl, struct blob_attr *m pattern = blobmsg_data(attr[EVREG_PATTERN]); len = strlen(pattern); - if (len <= 0) + if (!len) return UBUS_STATUS_PERMISSION_DENIED; if (pattern[len - 1] == '*') { diff --git a/ubusd_obj.c b/ubusd_obj.c index dd44882..6f274ce 100644 --- a/ubusd_obj.c +++ b/ubusd_obj.c @@ -37,7 +37,7 @@ static void ubus_unref_object_type(struct ubus_object_type *type) static bool ubus_create_obj_method(struct ubus_object_type *type, struct blob_attr *attr) { struct ubus_method *m; - int bloblen = blob_raw_len(attr); + size_t bloblen = blob_raw_len(attr); m = calloc(1, sizeof(*m) + bloblen); if (!m) diff --git a/ubusd_proto.c b/ubusd_proto.c index 92fbd55..cf955da 100644 --- a/ubusd_proto.c +++ b/ubusd_proto.c @@ -209,7 +209,7 @@ static int __ubusd_handle_lookup(struct ubus_client *cl, struct ubus_object *obj = NULL; char *objpath; bool found = false; - int len; + size_t len; if (!attr[UBUS_ATTR_OBJPATH]) { if (cmd) From 795b32bb96b611493f423666236e9c1e49e0736c Mon Sep 17 00:00:00 2001 From: Hauke Mehrtens Date: Thu, 21 May 2026 23:46:14 +0200 Subject: [PATCH 27/27] ubusd: use fixed-width types for sequence counters The message sequence number is a uint16_t on the wire (ubus_msghdr), and ubusd_acl_seq is emitted as a u32. Declaring the counters with the matching fixed-width types removes the implicit conversion that happened on every store or send: event_seq int -> uint16_t ubus_object.invoke_seq unsigned int -> uint16_t ubus_monitor.seq uint32_t -> uint16_t ubusd_acl_seq int -> uint32_t The values placed on the wire are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) Link: https://github.com/openwrt/ubus/pull/20 Signed-off-by: Hauke Mehrtens --- ubusd_acl.c | 2 +- ubusd_event.c | 2 +- ubusd_monitor.c | 2 +- ubusd_obj.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ubusd_acl.c b/ubusd_acl.c index 3cef2ee..438fa50 100644 --- a/ubusd_acl.c +++ b/ubusd_acl.c @@ -75,7 +75,7 @@ struct ubusd_acl_file { const char *ubusd_acl_dir = "/usr/share/acl.d"; static struct blob_buf bbuf; static struct avl_tree ubusd_acls; -static int ubusd_acl_seq; +static uint32_t ubusd_acl_seq; static struct ubus_object *acl_obj; static int diff --git a/ubusd_event.c b/ubusd_event.c index 47d89ca..34e9907 100644 --- a/ubusd_event.c +++ b/ubusd_event.c @@ -16,7 +16,7 @@ static struct avl_tree patterns; static struct ubus_object *event_obj; -static int event_seq = 0; +static uint16_t event_seq = 0; static int obj_event_seq = 1; struct event_source { diff --git a/ubusd_monitor.c b/ubusd_monitor.c index cad6b37..c09e0c1 100644 --- a/ubusd_monitor.c +++ b/ubusd_monitor.c @@ -19,7 +19,7 @@ static LIST_HEAD(monitors); struct ubus_monitor { struct list_head list; struct ubus_client *cl; - uint32_t seq; + uint16_t seq; }; static void diff --git a/ubusd_obj.h b/ubusd_obj.h index 5ed5ba8..a218187 100644 --- a/ubusd_obj.h +++ b/ubusd_obj.h @@ -56,7 +56,7 @@ struct ubus_object { const char *method, struct blob_attr *msg); int event_seen; - unsigned int invoke_seq; + uint16_t invoke_seq; }; struct ubus_object *ubusd_create_object(struct ubus_client *cl, struct blob_attr **attr);