Skip to content

Commit

Permalink
pimd: fix heap-use-after-free issue in igmp_source_delete()
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
patrasar committed Oct 4, 2023
1 parent 373d46d commit 33bb0c9
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 2 deletions.
1 change: 0 additions & 1 deletion pimd/pim6_mld.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,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;
}

Expand Down
1 change: 0 additions & 1 deletion pimd/pim_igmpv3.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
2 changes: 2 additions & 0 deletions pimd/pim_tib.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 33bb0c9

Please sign in to comment.