diff options
| author | Donald Sharp <sharpd@nvidia.com> | 2023-03-22 23:24:56 +0100 |
|---|---|---|
| committer | Donald Sharp <sharpd@nvidia.com> | 2023-03-22 23:29:56 +0100 |
| commit | e60308f498c356227df5dbc6cc04015a3b213f1d (patch) | |
| tree | af0f7fc57e9820bf971591e81676fb871d392905 /pimd/pim_zebra.c | |
| parent | Merge pull request #13006 from opensourcerouting/bgp-gr-null-warning (diff) | |
| download | frr-e60308f498c356227df5dbc6cc04015a3b213f1d.tar.xz frr-e60308f498c356227df5dbc6cc04015a3b213f1d.zip | |
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>
Diffstat (limited to 'pimd/pim_zebra.c')
| -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 29aac7f1c..f4e303286 100644 --- 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; } |