diff options
| author | Hauke Mehrtens | 2026-04-12 22:09:52 +0000 |
|---|---|---|
| committer | Hauke Mehrtens | 2026-05-23 00:47:14 +0000 |
| commit | 0c095592ccb79053dd02a57bf006dde2bd26ad28 (patch) | |
| tree | e81661599b85464ba162cd5598040e35d8078ed9 | |
| parent | 11ea1b3bdbea5ec5bd27fefff661a5f1b970d384 (diff) | |
| download | ubus-0c095592ccb79053dd02a57bf006dde2bd26ad28.tar.gz | |
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 <noreply@anthropic.com>
Link: https://github.com/openwrt/ubus/pull/20
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
| -rw-r--r-- | ubusd_proto.c | 9 |
1 files 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; |