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; diff --git a/libubus-io.c b/libubus-io.c index beea3bf..4d5f2e1 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); @@ -439,20 +448,26 @@ 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; - 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: free(buf); out_close: - if (ret) + if (ret) { close(ctx->sock.fd); + ctx->sock.fd = -1; + } return ret; } 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) diff --git a/libubus.c b/libubus.c index 5d22660..5f1c28c 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)); @@ -97,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: @@ -113,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; @@ -122,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) @@ -347,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; } @@ -438,9 +453,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) diff --git a/lua/ubus.c b/lua/ubus.c index 07b816d..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; } @@ -559,7 +566,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); } 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); diff --git a/ubusd_acl.c b/ubusd_acl.c index a5429cd..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 @@ -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; } @@ -303,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; @@ -316,6 +334,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; @@ -666,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; } diff --git a/ubusd_event.c b/ubusd_event.c index 3036969..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 { @@ -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] == '*') { @@ -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 { diff --git a/ubusd_id.c b/ubusd_id.c index 8d9fede..209cbb7 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,11 +72,10 @@ 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)) - 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; diff --git a/ubusd_main.c b/ubusd_main.c index 0e4c6d8..14d3be2 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,22 +248,55 @@ 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 void mkdir_sockdir() +static struct uloop_fd sighup_ufd = { + .cb = sighup_fd_cb, +}; + +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 @@ -272,12 +308,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': @@ -291,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); diff --git a/ubusd_monitor.c b/ubusd_monitor.c index bba741e..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 @@ -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) { 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_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); diff --git a/ubusd_proto.c b/ubusd_proto.c index 31263dc..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) @@ -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; @@ -552,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 */ @@ -583,10 +588,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 +599,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;