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 <sharpd@nvidia.com>

diff --git a/pimd/pim_zebra.c b/pimd/pim_zebra.c
index 29aac7f1c77d1cbd977382d9031aa1cc28735c4a..f4e30328677a43e1c7fa3adb24f0350128067dea 100644 (file)
--- a/pimd/pim_zebra.c
+++ b/pimd/pim_zebra.c
@@ -55,6 +55,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);
@@ -65,8 +67,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;
 }