diff options
| author | Donald Sharp <sharpd@nvidia.com> | 2023-03-22 18:24:56 -0400 |
|---|---|---|
| committer | Mergify <37929162+mergify[bot]@users.noreply.github.com> | 2023-03-24 13:03:32 +0000 |
| commit | ee065467729e36c2d514e01a52bb1d84b589fda6 (patch) | |
| tree | 34d836ce039b562ca67540bb72f25153c5bac810 | |
| parent | b157d367f10ca05e8e68846711c0450446892b0d (diff) | |
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>
(cherry picked from commit e60308f498c356227df5dbc6cc04015a3b213f1d)
| -rw-r--r-- | pimd/pim_zebra.c | 12 |
1 files changed, 12 insertions, 0 deletions
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; } |