bgpd: fix bgp_pbr_or_filter memory leaks

Note that bgp_pbr_policyroute_add_from_zebra() and
bgp_pbr_policyroute_remove_from_zebra() are only called from
bgp_pbr_handle_entry().

>  ==966967==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 40 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x7fd44744401a in list_new lib/linklist.c:49
>     #3 0x560f8c094490 in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2818
>     #4 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     #5 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     #6 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     #7 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     #8 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     #9 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     #10 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     #11 0x7fd4475779b2 in event_call lib/event.c:2011
>     #12 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     #13 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     #14 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>
> Direct leak of 40 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x7fd44744401a in list_new lib/linklist.c:49
>     #3 0x560f8c09439d in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2812
>     #4 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     #5 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     #6 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     #7 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     #8 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     #9 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     #10 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     #11 0x7fd4475779b2 in event_call lib/event.c:2011
>     #12 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     #13 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     #14 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>
> Direct leak of 4 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x560f8c080cec in bgp_pbr_extract_enumerate_unary bgpd/bgp_pbr.c:362
>     #3 0x560f8c080f7e in bgp_pbr_extract_enumerate bgpd/bgp_pbr.c:400
>     #4 0x560f8c094530 in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2819
>     #5 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     #6 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     #7 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     #8 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     #9 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     #10 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     #11 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     #12 0x7fd4475779b2 in event_call lib/event.c:2011
>     #13 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     #14 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     #15 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>
> Direct leak of 4 byte(s) in 1 object(s) allocated from:
>     #0 0x7fd447ab4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
>     #1 0x7fd44746f8dd in qcalloc lib/memory.c:105
>     #2 0x560f8c080cec in bgp_pbr_extract_enumerate_unary bgpd/bgp_pbr.c:362
>     #3 0x560f8c080f7e in bgp_pbr_extract_enumerate bgpd/bgp_pbr.c:400
>     #4 0x560f8c09443d in bgp_pbr_handle_entry bgpd/bgp_pbr.c:2813
>     #5 0x560f8c095993 in bgp_pbr_update_entry bgpd/bgp_pbr.c:2941
>     #6 0x560f8c2164f3 in bgp_zebra_announce bgpd/bgp_zebra.c:1618
>     #7 0x560f8c0bd668 in bgp_process_main_one bgpd/bgp_route.c:3691
>     #8 0x560f8c0be7fe in process_subq_other_route bgpd/bgp_route.c:3856
>     #9 0x560f8c0bf280 in process_subq bgpd/bgp_route.c:3955
>     #10 0x560f8c0bf320 in meta_queue_process bgpd/bgp_route.c:3980
>     #11 0x7fd44759fdfc in work_queue_run lib/workqueue.c:282
>     #12 0x7fd4475779b2 in event_call lib/event.c:2011
>     #13 0x7fd447442ff1 in frr_run lib/libfrr.c:1216
>     #14 0x560f8bef0a15 in main bgpd/bgp_main.c:545
>     #15 0x7fd446e29d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>

diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c
index 07de1e426e56f02e2055d9854bd7f404dc5af3fb..2b13715da3145b0ca20f20f2125812f2043e2fb2 100644 (file)
--- a/bgpd/bgp_pbr.c
+++ b/bgpd/bgp_pbr.c
@@ -279,6 +279,13 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp,
 
 static void bgp_pbr_dump_entry(struct bgp_pbr_filter *bpf, bool add);
 
+static void bgp_pbr_val_mask_free(void *arg)
+{
+       struct bgp_pbr_val_mask *pbr_val_mask = arg;
+
+       XFREE(MTYPE_PBR_VALMASK, pbr_val_mask);
+}
+
 static bool bgp_pbr_extract_enumerate_unary_opposite(
                                 uint8_t unary_operator,
                                 struct bgp_pbr_val_mask *and_valmask,
@@ -2130,17 +2137,6 @@ static void bgp_pbr_policyroute_remove_from_zebra(
                        bgp, path, bpf, bpof, FLOWSPEC_ICMP_TYPE);
        else
                bgp_pbr_policyroute_remove_from_zebra_unit(bgp, path, bpf);
-       /* flush bpof */
-       if (bpof->tcpflags)
-               list_delete_all_node(bpof->tcpflags);
-       if (bpof->dscp)
-               list_delete_all_node(bpof->dscp);
-       if (bpof->flowlabel)
-               list_delete_all_node(bpof->flowlabel);
-       if (bpof->pkt_len)
-               list_delete_all_node(bpof->pkt_len);
-       if (bpof->fragment)
-               list_delete_all_node(bpof->fragment);
 }
 
 static void bgp_pbr_dump_entry(struct bgp_pbr_filter *bpf, bool add)
@@ -2625,19 +2621,6 @@ static void bgp_pbr_policyroute_add_to_zebra(struct bgp *bgp,
                        bgp, path, bpf, bpof, nh, rate, FLOWSPEC_ICMP_TYPE);
        else
                bgp_pbr_policyroute_add_to_zebra_unit(bgp, path, bpf, nh, rate);
-       /* flush bpof */
-       if (bpof->tcpflags)
-               list_delete_all_node(bpof->tcpflags);
-       if (bpof->dscp)
-               list_delete_all_node(bpof->dscp);
-       if (bpof->pkt_len)
-               list_delete_all_node(bpof->pkt_len);
-       if (bpof->fragment)
-               list_delete_all_node(bpof->fragment);
-       if (bpof->icmp_type)
-               list_delete_all_node(bpof->icmp_type);
-       if (bpof->icmp_code)
-               list_delete_all_node(bpof->icmp_code);
 }
 
 static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
@@ -2703,6 +2686,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
                        srcp = ⦥
                else {
                        bpof.icmp_type = list_new();
+                       bpof.icmp_type->del = bgp_pbr_val_mask_free;
                        bgp_pbr_extract_enumerate(api->icmp_type,
                                                  api->match_icmp_type_num,
                                                  OPERATOR_UNARY_OR,
@@ -2718,6 +2702,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
                        dstp = &range_icmp_code;
                else {
                        bpof.icmp_code = list_new();
+                       bpof.icmp_code->del = bgp_pbr_val_mask_free;
                        bgp_pbr_extract_enumerate(api->icmp_code,
                                                  api->match_icmp_code_num,
                                                  OPERATOR_UNARY_OR,
@@ -2738,6 +2723,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
                                                  FLOWSPEC_TCP_FLAGS);
                } else if (kind_enum == OPERATOR_UNARY_OR) {
                        bpof.tcpflags = list_new();
+                       bpof.tcpflags->del = bgp_pbr_val_mask_free;
                        bgp_pbr_extract_enumerate(api->tcpflags,
                                                  api->match_tcpflags_num,
                                                  OPERATOR_UNARY_OR,
@@ -2755,6 +2741,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
                        bpf.pkt_len = &pkt_len;
                else {
                        bpof.pkt_len = list_new();
+                       bpof.pkt_len->del = bgp_pbr_val_mask_free;
                        bgp_pbr_extract_enumerate(api->packet_length,
                                                  api->match_packet_length_num,
                                                  OPERATOR_UNARY_OR,
@@ -2764,12 +2751,14 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
        }
        if (api->match_dscp_num >= 1) {
                bpof.dscp = list_new();
+               bpof.dscp->del = bgp_pbr_val_mask_free;
                bgp_pbr_extract_enumerate(api->dscp, api->match_dscp_num,
                                          OPERATOR_UNARY_OR,
                                          bpof.dscp, FLOWSPEC_DSCP);
        }
        if (api->match_fragment_num) {
                bpof.fragment = list_new();
+               bpof.fragment->del = bgp_pbr_val_mask_free;
                bgp_pbr_extract_enumerate(api->fragment,
                                          api->match_fragment_num,
                                          OPERATOR_UNARY_OR,
@@ -2785,7 +2774,7 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
        bpf.family = afi2family(api->afi);
        if (!add) {
                bgp_pbr_policyroute_remove_from_zebra(bgp, path, &bpf, &bpof);
-               return;
+               goto flush_bpof;
        }
        /* no action for add = true */
        for (i = 0; i < api->action_num; i++) {
@@ -2863,6 +2852,22 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path,
                if (continue_loop == 0)
                        break;
        }
+
+flush_bpof:
+       if (bpof.tcpflags)
+               list_delete(&bpof.tcpflags);
+       if (bpof.dscp)
+               list_delete(&bpof.dscp);
+       if (bpof.flowlabel)
+               list_delete(&bpof.flowlabel);
+       if (bpof.pkt_len)
+               list_delete(&bpof.pkt_len);
+       if (bpof.fragment)
+               list_delete(&bpof.fragment);
+       if (bpof.icmp_type)
+               list_delete(&bpof.icmp_type);
+       if (bpof.icmp_code)
+               list_delete(&bpof.icmp_code);
 }
 
 void bgp_pbr_update_entry(struct bgp *bgp, const struct prefix *p,