proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-04-28 15:36:25 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

pimd: Fix use after free issue for ifp's moving vrfs

Browse Source 
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>
This commit is contained in:
Donald Sharp 2023-03-22 18:24:56 -04:00
parent 9a02d9a3bc
commit e60308f498
1 changed files with 12 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
pimd/pim_zebra.c
View File

@ -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;
}