From a5d7ceb6aaf2bff5beca3f8588264192f3e43994 Mon Sep 17 00:00:00 2001 From: Ales Musil Date: Thu, 4 Jan 2024 13:54:24 +0100 Subject: [PATCH] treewide: Fix small memory leaks reported by static analysis Fix a couple of small memory leaks that were reported by coverity static analysis on ovn rpm package. Signed-off-by: Ales Musil Acked-by: Mark Michelson Signed-off-by: Dumitru Ceara --- controller-vtep/ovn-controller-vtep.c | 2 + controller/lflow.c | 1 + controller/ovn-controller.c | 1 + controller/pinctrl.c | 60 +++++++++++++++------------ utilities/ovn-nbctl.c | 1 + utilities/ovn-trace.c | 1 + 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/controller-vtep/ovn-controller-vtep.c b/controller-vtep/ovn-controller-vtep.c index 23b368179a..4472e52853 100644 --- a/controller-vtep/ovn-controller-vtep.c +++ b/controller-vtep/ovn-controller-vtep.c @@ -306,10 +306,12 @@ parse_options(int argc, char *argv[]) switch (c) { case 'd': + free(ovnsb_remote); ovnsb_remote = xstrdup(optarg); break; case 'D': + free(vtep_remote); vtep_remote = xstrdup(optarg); break; diff --git a/controller/lflow.c b/controller/lflow.c index 85a4943dc6..b0cf4253c4 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -1017,6 +1017,7 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow, static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "error parsing match \"%s\": %s", lflow->match, error); + expr_destroy(e); free(error); return NULL; } diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 8709c1ae2a..856e5e2708 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -6180,6 +6180,7 @@ parse_options(int argc, char *argv[]) break; case 'n': + free(cli_system_id); cli_system_id = xstrdup(optarg); break; diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 12055a6756..9ff44cf5da 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -6401,26 +6401,31 @@ pinctrl_handle_empty_lb_backends_opts(struct ofpbuf *userdata) char *protocol = NULL; char *load_balancer = NULL; + struct empty_lb_backends_event *event = NULL; + while (userdata->size) { userdata_opt = ofpbuf_try_pull(userdata, sizeof opt_hdr); if (!userdata_opt) { - return false; + goto cleanup; } memcpy(&opt_hdr, userdata_opt, sizeof opt_hdr); size_t size = ntohs(opt_hdr.size); char *userdata_opt_data = ofpbuf_try_pull(userdata, size); if (!userdata_opt_data) { - return false; + goto cleanup; } switch (ntohs(opt_hdr.opt_code)) { case EMPTY_LB_VIP: + free(vip); vip = xmemdup0(userdata_opt_data, size); break; case EMPTY_LB_PROTOCOL: + free(protocol); protocol = xmemdup0(userdata_opt_data, size); break; case EMPTY_LB_LOAD_BALANCER: + free(load_balancer); load_balancer = xmemdup0(userdata_opt_data, size); break; default: @@ -6431,36 +6436,39 @@ pinctrl_handle_empty_lb_backends_opts(struct ofpbuf *userdata) if (!vip || !protocol || !load_balancer) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); VLOG_WARN_RL(&rl, "missing lb parameters in userdata"); - free(vip); - free(protocol); - free(load_balancer); - return false; + goto cleanup; } - struct empty_lb_backends_event *event; - event = pinctrl_find_empty_lb_backends_event(vip, protocol, load_balancer, hash); - if (!event) { - if (hmap_count(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS]) >= 1000) { - COVERAGE_INC(pinctrl_drop_controller_event); - return false; - } + if (event) { + goto cleanup; + } - event = xzalloc(sizeof *event); - hmap_insert(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS], - &event->hmap_node, hash); - event->vip = vip; - event->protocol = protocol; - event->load_balancer = load_balancer; - event->timestamp = time_msec(); - notify_pinctrl_main(); - } else { - free(vip); - free(protocol); - free(load_balancer); + if (hmap_count(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS]) >= 1000) { + COVERAGE_INC(pinctrl_drop_controller_event); + goto cleanup; } - return true; + + event = xzalloc(sizeof *event); + hmap_insert(&event_table[OVN_EVENT_EMPTY_LB_BACKENDS], + &event->hmap_node, hash); + event->vip = vip; + event->protocol = protocol; + event->load_balancer = load_balancer; + event->timestamp = time_msec(); + notify_pinctrl_main(); + + vip = NULL; + protocol = NULL; + load_balancer = NULL; + +cleanup: + free(vip); + free(protocol); + free(load_balancer); + + return event != NULL; } static void diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 9522078c18..526369b68a 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -4694,6 +4694,7 @@ static void nexthop = normalize_prefix_str(ctx->argv[3]); if (!nexthop) { ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]); + free(prefix); return; } } diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index 0b86eae7be..13ae464ad2 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -983,6 +983,7 @@ parse_lflow_for_datapath(const struct sbrec_logical_flow *sblf, if (error) { VLOG_WARN("%s: parsing expression failed (%s)", sblf->match, error); + expr_destroy(match); free(error); return; }