From d8d58e98397d8442ec68f8d715b64d5e6000b903 Mon Sep 17 00:00:00 2001 From: Hung-Weic Chiu Date: Sat, 29 Apr 2017 14:20:15 +0000 Subject: [PATCH 1/5] Fix the memory leak - free the memory for all cases. Signed-off-by: Hung-Weic Chiu --- lib/csv.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/csv.c b/lib/csv.c index 7df9292647..95e3e97768 100644 --- a/lib/csv.c +++ b/lib/csv.c @@ -177,6 +177,9 @@ csv_decode_record(csv_record_t *rec) field = strpbrk(curr, ","); } field = strstr(curr, "\n"); + if (!field) { + return; + } fld = malloc(sizeof(csv_field_t)); if (field && fld) { fld->field = curr; @@ -239,6 +242,10 @@ csv_encode (csv_t *csv, rec = malloc(sizeof(csv_record_t)); if (!rec) { log_error("record malloc failed\n"); + if (!buf) { + free(str); + } + va_end(list); return (NULL); } csv_init_record(rec); From 5d6cc38ca36538583ff4c464c46a7c6de08608b6 Mon Sep 17 00:00:00 2001 From: Hung-Weic Chiu Date: Sat, 29 Apr 2017 15:02:31 +0000 Subject: [PATCH 2/5] Fix the "Use-after-free" of clang SA. - Set the pointer to NULL after free it, otherwise the pointer will be accessed again. (since not null) Signed-off-by: Hung-Weic Chiu --- lib/imsg-buffer.c | 4 +++- lib/imsg.c | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/imsg-buffer.c b/lib/imsg-buffer.c index a486fc17c1..f7b9f63778 100644 --- a/lib/imsg-buffer.c +++ b/lib/imsg-buffer.c @@ -209,8 +209,10 @@ msgbuf_clear(struct msgbuf *msgbuf) { struct ibuf *buf; - while ((buf = TAILQ_FIRST(&msgbuf->bufs)) != NULL) + while ((buf = TAILQ_FIRST(&msgbuf->bufs)) != NULL) { ibuf_dequeue(msgbuf, buf); + TAILQ_FIRST(&msgbuf->bufs) = NULL; + } } int diff --git a/lib/imsg.c b/lib/imsg.c index 246430cdd5..df3cdf510c 100644 --- a/lib/imsg.c +++ b/lib/imsg.c @@ -310,6 +310,7 @@ imsg_get_fd(struct imsgbuf *ibuf) fd = ifd->fd; TAILQ_REMOVE(&ibuf->fds, ifd, entry); free(ifd); + TAILQ_FIRST(&ibuf->fds) = NULL; return (fd); } From 3a6570a1f145c49155d72a815441025085dd45ad Mon Sep 17 00:00:00 2001 From: Hung-Weic Chiu Date: Sat, 29 Apr 2017 15:25:32 +0000 Subject: [PATCH 3/5] Fix the "Dead assignment" of clang SA. - Remove duplicated assignemt. - Remove unused initialized. Signed-off-by: Hung-Weic Chiu --- ospfd/ospf_apiserver.c | 2 +- ospfd/ospf_vty.c | 9 --------- pimd/pim_register.c | 4 ++-- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/ospfd/ospf_apiserver.c b/ospfd/ospf_apiserver.c index aac8ef4b8b..620dacb157 100644 --- a/ospfd/ospf_apiserver.c +++ b/ospfd/ospf_apiserver.c @@ -2459,7 +2459,7 @@ ospf_apiserver_clients_notify_nsm_change (struct ospf_neighbor *nbr) { struct msg *msg; struct in_addr ifaddr = { .s_addr = 0L }; - struct in_addr nbraddr = { .s_addr = 0L }; + struct in_addr nbraddr; assert (nbr); diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 5faf2850d9..38e006e928 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -6758,7 +6758,6 @@ DEFUN (no_ip_ospf_cost, int ret; struct ospf_if_params *params; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); if (argc == 1) @@ -6827,7 +6826,6 @@ DEFUN (no_ip_ospf_cost2, int ret; struct ospf_if_params *params; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); /* According to the semantics we are mimicking "no ip ospf cost N" is @@ -7067,7 +7065,6 @@ DEFUN (no_ip_ospf_dead_interval, struct ospf_interface *oi; struct route_node *rn; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); if (argc == 2) @@ -7242,7 +7239,6 @@ DEFUN (no_ip_ospf_hello_interval, int ret; struct ospf_if_params *params; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); if (argc == 2) @@ -7520,7 +7516,6 @@ DEFUN (no_ip_ospf_priority, int ret; struct ospf_if_params *params; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); if (argc == 2) @@ -7667,7 +7662,6 @@ DEFUN (no_ip_ospf_retransmit_interval, struct ospf_if_params *params; int addr_index; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); if (argc >= 1) @@ -7739,7 +7733,6 @@ DEFUN (no_ip_ospf_retransmit_interval_sec, struct interface *ifp = vty->index; struct ospf_if_params *params; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); UNSET_IF_PARAM (params, retransmit_interval); @@ -7824,7 +7817,6 @@ DEFUN (no_ip_ospf_transmit_delay, struct ospf_if_params *params; int addr_index; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); if (argc >= 1) @@ -7897,7 +7889,6 @@ DEFUN (no_ip_ospf_transmit_delay_sec, struct interface *ifp = vty->index; struct ospf_if_params *params; - ifp = vty->index; params = IF_DEF_PARAMS (ifp); UNSET_IF_PARAM (params, transmit_delay); diff --git a/pimd/pim_register.c b/pimd/pim_register.c index ce3ac1a433..7844bd3399 100644 --- a/pimd/pim_register.c +++ b/pimd/pim_register.c @@ -160,8 +160,8 @@ pim_register_recv (struct interface *ifp, int sentRegisterStop = 0; struct ip *ip_hdr; //size_t hlen; - struct in_addr group = { .s_addr = 0 }; - struct in_addr source = { .s_addr = 0 }; + struct in_addr group; + struct in_addr source; //uint8_t *msg; uint32_t *bits; From c604467a081a332848bf486a207649d79abbe3fd Mon Sep 17 00:00:00 2001 From: Hung-Weic Chiu Date: Sat, 29 Apr 2017 15:34:18 +0000 Subject: [PATCH 4/5] Fix the "Uninitialized argument value" of clang SA. Signed-off-by: Hung-Weic Chiu --- cumulus/start-stop-daemon.c | 1 + ospfd/ospf_vty.c | 2 +- zebra/zserv.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cumulus/start-stop-daemon.c b/cumulus/start-stop-daemon.c index 4d447d9051..a3d3c3dd33 100644 --- a/cumulus/start-stop-daemon.c +++ b/cumulus/start-stop-daemon.c @@ -811,6 +811,7 @@ run_stop_schedule(void) anykilled = 0; retry_nr = 0; + n_killed = 0; if (schedule == NULL) { do_stop(signal_nr, quietmode, &n_killed, &n_notkilled, 0); diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 38e006e928..2724df98a0 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -7059,7 +7059,7 @@ DEFUN (no_ip_ospf_dead_interval, "Address of interface") { struct interface *ifp = vty->index; - struct in_addr addr; + struct in_addr addr = { .s_addr = 0L}; int ret; struct ospf_if_params *params; struct ospf_interface *oi; diff --git a/zebra/zserv.c b/zebra/zserv.c index 8618e5c371..39fc226ea7 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -1519,7 +1519,7 @@ zread_ipv6_delete (struct zserv *client, u_short length, struct zebra_vrf *zvrf) struct stream *s; struct zapi_ipv6 api; struct in6_addr nexthop; - union g_addr *pnexthop; + union g_addr *pnexthop = NULL; unsigned long ifindex; struct prefix p; From 4da93320c7332aaa894c9ec32cff43984adba60d Mon Sep 17 00:00:00 2001 From: Hung-Weic Chiu Date: Tue, 2 May 2017 00:28:41 +0000 Subject: [PATCH 5/5] Revert "Fix the "Use-after-free" of clang SA." - This's the wrong way to fix this problem. - Since the "TAILQ_FIRST()" always return diferent pointer as we call "TAILQ_REMOVE()", the clang static analyzer can't detect this behavior. - Ignore this warning and keep files identical to its original one. This reverts commit 5d6cc38ca36538583ff4c464c46a7c6de08608b6. Signed-off-by: Hung-Weic Chiu --- lib/imsg-buffer.c | 4 +--- lib/imsg.c | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/imsg-buffer.c b/lib/imsg-buffer.c index f7b9f63778..a486fc17c1 100644 --- a/lib/imsg-buffer.c +++ b/lib/imsg-buffer.c @@ -209,10 +209,8 @@ msgbuf_clear(struct msgbuf *msgbuf) { struct ibuf *buf; - while ((buf = TAILQ_FIRST(&msgbuf->bufs)) != NULL) { + while ((buf = TAILQ_FIRST(&msgbuf->bufs)) != NULL) ibuf_dequeue(msgbuf, buf); - TAILQ_FIRST(&msgbuf->bufs) = NULL; - } } int diff --git a/lib/imsg.c b/lib/imsg.c index df3cdf510c..246430cdd5 100644 --- a/lib/imsg.c +++ b/lib/imsg.c @@ -310,7 +310,6 @@ imsg_get_fd(struct imsgbuf *ibuf) fd = ifd->fd; TAILQ_REMOVE(&ibuf->fds, ifd, entry); free(ifd); - TAILQ_FIRST(&ibuf->fds) = NULL; return (fd); }