From 8d0fdbdadd157d58eb16eb3206e6dd68a3890a8f Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 30 May 2025 11:24:18 +0800 Subject: [PATCH 1/4] mctpd: Release sdbus refs on exit Previously the sd_bus and associated objects were not freed on exit, so valgrind would report leaks. This doesn't matter itself, but makes it difficult to detect real leaks. Signed-off-by: Matt Johnston --- src/mctpd.c | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index f19a763a..f658188c 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -1494,6 +1494,9 @@ static void free_peers(struct ctx *ctx) free(peer->message_types); free(peer->uuid); free(peer->path); + sd_bus_slot_unref(peer->slot_obmc_endpoint); + sd_bus_slot_unref(peer->slot_cc_endpoint); + sd_bus_slot_unref(peer->slot_uuid); free(peer); } @@ -3245,6 +3248,13 @@ static int prune_old_nets(struct ctx *ctx) return 0; } +static void free_link(struct link *link) { + sd_bus_slot_unref(link->slot_iface); + sd_bus_slot_unref(link->slot_busowner); + free(link->path); + free(link); +} + // Removes remote peers associated with an old interface. // Note that this link has already been removed from ctx->nl */ static int del_interface(struct link *link) @@ -3269,17 +3279,28 @@ static int del_interface(struct link *link) warnx("Failed to remove D-Bus interface of ifindex %d", link->ifindex); prune_old_nets(ctx); + free_link(link); - sd_bus_slot_unref(link->slot_iface); - link->slot_iface = NULL; - sd_bus_slot_unref(link->slot_busowner); - link->slot_busowner = NULL; - - free(link->path); - free(link); return 0; } +// For program termination cleanup +static void free_links(struct ctx *ctx) +{ + size_t num; + int* ifs; + + ifs = mctp_nl_if_list(ctx->nl, &num); + for (size_t i = 0; i < num; i++) { + struct link* link = mctp_nl_get_link_userdata(ctx->nl, ifs[i]); + mctp_nl_set_link_userdata(ctx->nl, ifs[i], NULL); + if (link) { + free_link(link); + } + } + free(ifs); +} + // Moves remote peers from old->new net. static int change_net_interface(struct ctx *ctx, int ifindex, uint32_t old_net) { @@ -3865,13 +3886,13 @@ int main(int argc, char **argv) return 1; } - sd_bus_flush(ctx->bus); - sd_bus_close(ctx->bus); - - mctp_nl_close(ctx->nl); + sd_bus_flush_close_unrefp(&ctx->bus); + free_links(ctx); free_peers(ctx); free_nets(ctx); + mctp_nl_close(ctx->nl); + return 0; } From 64c41d7e385198954d98e96ba4a1ec5423b523ef Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 30 May 2025 11:26:32 +0800 Subject: [PATCH 2/4] mctpd: Use calloc rather than malloc+memset Signed-off-by: Matt Johnston --- src/mctpd.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index f658188c..7a47e852 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -1378,12 +1378,10 @@ static int add_peer(struct ctx *ctx, const dest_phys *dest, mctp_eid_t eid, return -ENOMEM; // Allocate the peer itself - peer = malloc(sizeof(*peer)); + peer = calloc(1, sizeof(*peer)); if (!peer) return -ENOMEM; - memset(peer, 0, sizeof(*peer)); - // Add it to our peers array tmp = realloc(ctx->peers, (ctx->num_peers + 1) * sizeof(*ctx->peers)); if (!tmp) @@ -3501,12 +3499,11 @@ static int add_net(struct ctx *ctx, uint32_t net_id) return -EEXIST; } - net = malloc(sizeof(*net)); + net = calloc(1, sizeof(*net)); if (!net) { warn("failed to allocate net"); return -ENOMEM; } - memset(net, 0, sizeof(*net)); // Initialise the new entry net->net = net_id; From 0b6cdf6b543ecbeb0811d9a87ccb4d0a3fdfeb43 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 30 May 2025 11:26:44 +0800 Subject: [PATCH 3/4] mctpd: Avoid realloc(size=0) This doesn't change program behaviour, but avoids a noisy warning from valgrind. Signed-off-by: Matt Johnston --- src/mctpd.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index 7a47e852..0104e29d 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -1472,12 +1472,17 @@ static int remove_peer(struct peer *peer) memmove(ctx->peers + idx, ctx->peers + idx + 1, (ctx->num_peers - idx) * sizeof(struct peer *)); - tmp = realloc(ctx->peers, ctx->num_peers * sizeof(struct peer *)); - if (!tmp && ctx->num_peers) { - warn("%s: peer realloc(reduce!) failed", __func__); - // we'll re-try on next add/remove + if (ctx->num_peers > 0) { + tmp = realloc(ctx->peers, ctx->num_peers * sizeof(struct peer *)); + if (!tmp) { + warn("%s: peer realloc(reduce!) failed", __func__); + // we'll re-try on next add/remove + } else { + ctx->peers = tmp; + } } else { - ctx->peers = tmp; + free(ctx->peers); + ctx->peers = NULL; } free(peer); From ee23fcfe3a7be548aa8677de17b1b5a4b957aa48 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 30 May 2025 11:46:00 +0800 Subject: [PATCH 4/4] mctpd: Be cautious of deleting unknown links This may not be possible (hasn't been encountered), but check in case. Signed-off-by: Matt Johnston --- src/mctpd.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/mctpd.c b/src/mctpd.c index 0104e29d..3c798ace 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -958,7 +958,14 @@ static int cb_listen_monitor(sd_event_source *s, int sd, uint32_t revents, case MCTP_NL_DEL_LINK: { // Local addresses have already been deleted with DEL_EID - rc = del_interface(c->link_userdata); + if (c->link_userdata) { + rc = del_interface(c->link_userdata); + } else { + // Would have expected to have seen it in previous + // MCTP_NL_ADD_LINK or setup_nets(). + rc = -ENOENT; + bug_warn("delete unconfigured interface %d", c->ifindex); + } any_error |= (rc < 0); } break;