From ee065467729e36c2d514e01a52bb1d84b589fda6 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 22 Mar 2023 18:24:56 -0400 Subject: [PATCH] pimd: Fix use after free issue for ifp's moving vrfs We have this valgrind trace: ==1125== Invalid read of size 4 ==1125== at 0x170A7D: pim_if_delete (pim_iface.c:203) ==1125== by 0x170C01: pim_if_terminate (pim_iface.c:80) ==1125== by 0x174F34: pim_instance_terminate (pim_instance.c:68) ==1125== by 0x17535B: pim_vrf_terminate (pim_instance.c:260) ==1125== by 0x1941CF: pim_terminate (pimd.c:161) ==1125== by 0x1B476D: pim_sigint (pim_signals.c:44) ==1125== by 0x4910C22: frr_sigevent_process (sigevent.c:133) ==1125== by 0x49220A4: thread_fetch (thread.c:1777) ==1125== by 0x48DC8E2: frr_run (libfrr.c:1222) ==1125== by 0x15E12A: main (pim_main.c:176) ==1125== Address 0x6274d28 is 1,192 bytes inside a block of size 1,752 free'd ==1125== at 0x48369AB: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==1125== by 0x174FF1: pim_vrf_delete (pim_instance.c:181) ==1125== by 0x4925480: vrf_delete (vrf.c:264) ==1125== by 0x4925480: vrf_delete (vrf.c:238) ==1125== by 0x49332C7: zclient_vrf_delete (zclient.c:2187) ==1125== by 0x4934319: zclient_read (zclient.c:4003) ==1125== by 0x492249C: thread_call (thread.c:2008) ==1125== by 0x48DC8D7: frr_run (libfrr.c:1223) ==1125== by 0x15E12A: main (pim_main.c:176) ==1125== Block was alloc'd at ==1125== at 0x4837B65: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==1125== by 0x48E80AF: qcalloc (memory.c:116) ==1125== by 0x1750DA: pim_instance_init (pim_instance.c:90) ==1125== by 0x1750DA: pim_vrf_new (pim_instance.c:161) ==1125== by 0x4924FDC: vrf_get (vrf.c:183) ==1125== by 0x493334C: zclient_vrf_add (zclient.c:2157) ==1125== by 0x4934319: zclient_read (zclient.c:4003) ==1125== by 0x492249C: thread_call (thread.c:2008) ==1125== by 0x48DC8D7: frr_run (libfrr.c:1223) ==1125== by 0x15E12A: main (pim_main.c:176) and you do this series of events: a) Create a vrf, put an interface in it b) Turn on pim on that interface and turn on pim in that vrf c) Delete the vrf d) Do anything with the interface, in this case shutdown the system The move of the interface to a new vrf is leaving the pim_ifp->pim pointer pointing at the old pim instance, which was just deleted, so the instance pointer was freed. Let's clean up the pim pointer in the interface pointer as well. Signed-off-by: Donald Sharp (cherry picked from commit e60308f498c356227df5dbc6cc04015a3b213f1d) --- pimd/pim_zebra.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pimd/pim_zebra.c b/pimd/pim_zebra.c index 6377deca21..bb9f527649 100644 --- a/pimd/pim_zebra.c +++ b/pimd/pim_zebra.c @@ -68,6 +68,8 @@ static int pim_zebra_interface_vrf_update(ZAPI_CALLBACK_ARGS) { struct interface *ifp; vrf_id_t new_vrf_id; + struct pim_instance *pim; + struct pim_interface *pim_ifp; ifp = zebra_interface_vrf_update_read(zclient->ibuf, vrf_id, &new_vrf_id); @@ -78,8 +80,18 @@ static int pim_zebra_interface_vrf_update(ZAPI_CALLBACK_ARGS) zlog_debug("%s: %s updating from %u to %u", __func__, ifp->name, vrf_id, new_vrf_id); + pim = pim_get_pim_instance(new_vrf_id); + if_update_to_new_vrf(ifp, new_vrf_id); + pim_ifp = ifp->info; + if (!pim_ifp) + return 0; + + pim_ifp->pim->mcast_if_count--; + pim_ifp->pim = pim; + pim_ifp->pim->mcast_if_count++; + return 0; } -- 2.39.5