From 8e4e2fa5ede9a17bcbc5578fb84749ec4b767dab Mon Sep 17 00:00:00 2001 From: Sarita Patra Date: Wed, 16 Aug 2023 01:32:25 -0700 Subject: [PATCH] pimd: fix heap-use-after-free issue in igmp_source_delete() ERROR: AddressSanitizer: heap-use-after-free on address 0x6160000801f0 at pc 0x5598c27c213f bp 0x7ffc04462060 sp 0x7ffc04462050 READ of size 4 at 0x6160000801f0 thread T0 #0 0x5598c27c213e in igmp_source_delete pimd/pim_igmpv3.c:340 #1 0x5598c27c277f in igmp_source_delete_expired pimd/pim_igmpv3.c:405 #2 0x5598c27b34c7 in igmp_group_timer pimd/pim_igmp.c:1324 #3 0x7fb78e68e1a7 in event_call lib/event.c:1995 #4 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213 #5 0x5598c27c781d in main pimd/pim_main.c:162 #6 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86) #7 0x5598c26f0ab9 in _start (/usr/lib/frr/pimd+0x103ab9) 0x6160000801f0 is located 112 bytes inside of 600-byte region [0x616000080180,0x6160000803d8) freed by thread T0 here: #0 0x7fb78ebf17a8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7a8) #1 0x7fb78e5eff7c in qfree lib/memory.c:130 #2 0x5598c2750412 in pim_channel_oil_free pimd/pim_oil.c:84 #3 0x5598c2750c26 in pim_channel_oil_del pimd/pim_oil.c:199 #4 0x5598c27681d3 in tib_sg_gm_prune pimd/pim_tib.c:167 #5 0x5598c27b0e7f in igmp_source_forward_stop pimd/pim_igmp.c:225 #6 0x5598c27c25ac in igmp_source_timer pimd/pim_igmpv3.c:155 #7 0x7fb78e68e1a7 in event_call lib/event.c:1995 #8 0x7fb78e5d28a5 in frr_run lib/libfrr.c:1213 #9 0x5598c27c781d in main pimd/pim_main.c:162 #10 0x7fb78dbeac86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86) Root Cause: ----------- This issue arises because the "source->source_channel_oil" pointer is deleted within the "igmp_source_forward_stop()" API. However when the igmp_source_delete function in pimd/pim_igmpv3.c at line 340 is called, it attempts to reference the "source->source_channel_oil" pointer, which now containes garbage value. Fix: ---- After deletion of "source_channel_oil" in igmp_source_forward_stop() API, it is necessary to set the "source->source_channel_oil" pointer to NULL value. This will prevent any issue that could arise from referencing in future. Issue: #14195 Signed-off-by: Sarita Patra --- pimd/pim6_mld.c | 1 - pimd/pim_igmpv3.c | 1 - pimd/pim_tib.c | 2 ++ 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pimd/pim6_mld.c b/pimd/pim6_mld.c index a39d182990bf..6cb94035adaa 100644 --- a/pimd/pim6_mld.c +++ b/pimd/pim6_mld.c @@ -437,7 +437,6 @@ static void gm_sg_update(struct gm_sg *sg, bool has_expired) } else if (sg->tib_joined && !new_join) { tib_sg_gm_prune(gm_ifp->pim, sg->sgaddr, gm_ifp->ifp, &sg->oil); - sg->oil = NULL; sg->tib_joined = false; } diff --git a/pimd/pim_igmpv3.c b/pimd/pim_igmpv3.c index 2c5ad4d44b1b..ef8195d1cfef 100644 --- a/pimd/pim_igmpv3.c +++ b/pimd/pim_igmpv3.c @@ -347,7 +347,6 @@ void igmp_source_delete(struct gm_source *source) source_timer_off(group, source); igmp_source_forward_stop(source); - source->source_channel_oil = NULL; /* sanity check that forwarding has been disabled */ if (IGMP_SOURCE_TEST_FORWARDING(source->source_flags)) { diff --git a/pimd/pim_tib.c b/pimd/pim_tib.c index 4081786c1e49..5eabd1f1c9c1 100644 --- a/pimd/pim_tib.c +++ b/pimd/pim_tib.c @@ -165,4 +165,6 @@ void tib_sg_gm_prune(struct pim_instance *pim, pim_sgaddr sg, pim_ifchannel_local_membership_del(oif, &sg); pim_channel_oil_del(*oilp, __func__); + + *oilp = NULL; }