From d0438da6b09333d2b77a9eac2e9fffbbae6e603b Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 24 Mar 2022 20:02:33 -0400 Subject: zebra: Fix use after deletion event in freebsd In the FreeBSD code if you delete the interface and it has no configuration, the ifp pointer will be deleted from the system *but* zebra continues to dereference the just freed pointer. ==58624== Invalid read of size 1 ==58624== at 0x48539F3: strlcpy (in /usr/local/libexec/valgrind/vgpreload_memcheck-amd64-freebsd.so) ==58624== by 0x2B0565: ifreq_set_name (ioctl.c:48) ==58624== by 0x2B0565: if_get_flags (ioctl.c:416) ==58624== by 0x2B2D9E: ifan_read (kernel_socket.c:455) ==58624== by 0x2B2D9E: kernel_read (kernel_socket.c:1403) ==58624== by 0x499F46E: thread_call (thread.c:2002) ==58624== by 0x495D2B7: frr_run (libfrr.c:1196) ==58624== by 0x2B40B8: main (main.c:471) ==58624== Address 0x6baa7f0 is 64 bytes inside a block of size 432 free'd ==58624== at 0x484ECDC: free (in /usr/local/libexec/valgrind/vgpreload_memcheck-amd64-freebsd.so) ==58624== by 0x4953A64: if_delete (if.c:283) ==58624== by 0x2A93C1: if_delete_update (interface.c:874) ==58624== by 0x2B2DF3: ifan_read (kernel_socket.c:453) ==58624== by 0x2B2DF3: kernel_read (kernel_socket.c:1403) ==58624== by 0x499F46E: thread_call (thread.c:2002) ==58624== by 0x495D2B7: frr_run (libfrr.c:1196) ==58624== by 0x2B40B8: main (main.c:471) ==58624== Block was alloc'd at ==58624== at 0x4851381: calloc (in /usr/local/libexec/valgrind/vgpreload_memcheck-amd64-freebsd.so) ==58624== by 0x496A022: qcalloc (memory.c:116) ==58624== by 0x49546BC: if_new (if.c:164) ==58624== by 0x49546BC: if_create_name (if.c:218) ==58624== by 0x49546BC: if_get_by_name (if.c:603) ==58624== by 0x2B1295: ifm_read (kernel_socket.c:628) ==58624== by 0x2A7FB6: interface_list (if_sysctl.c:129) ==58624== by 0x2E99C8: zebra_ns_enable (zebra_ns.c:127) ==58624== by 0x2E99C8: zebra_ns_init (zebra_ns.c:214) ==58624== by 0x2B3FF2: main (main.c:401) ==58624== Zebra needs to pass back whether or not the ifp pointer was freed when if_delete_update is called and it should then check in ifan_read as well as ifm_read that the ifp pointer is still valid for use. Signed-off-by: Donald Sharp --- zebra/kernel_socket.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) (limited to 'zebra/kernel_socket.c') diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index 6583af0a54..0d2830a39c 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -450,12 +450,13 @@ static int ifan_read(struct if_announcemsghdr *ifan) if_get_metric(ifp); if_add_update(ifp); } else if (ifp != NULL && ifan->ifan_what == IFAN_DEPARTURE) - if_delete_update(ifp); - - if_get_flags(ifp); - if_get_mtu(ifp); - if_get_metric(ifp); + if_delete_update(&ifp); + if (ifp) { + if_get_flags(ifp); + if_get_mtu(ifp); + if_get_metric(ifp); + } if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug("%s: interface %s index %d", __func__, ifan->ifan_name, ifan->ifan_index); @@ -722,10 +723,10 @@ int ifm_read(struct if_msghdr *ifm) * will still behave correctly if run on a platform * without */ - if_delete_update(ifp); + if_delete_update(&ifp); } #endif /* RTM_IFANNOUNCE */ - if (if_is_up(ifp)) { + if (ifp && if_is_up(ifp)) { #if defined(__bsdi__) if_kvm_get_mtu(ifp); #else @@ -735,14 +736,16 @@ int ifm_read(struct if_msghdr *ifm) } } + if (ifp) { #ifdef HAVE_NET_RT_IFLIST - ifp->stats = ifm->ifm_data; + ifp->stats = ifm->ifm_data; #endif /* HAVE_NET_RT_IFLIST */ - ifp->speed = ifm->ifm_data.ifi_baudrate / 1000000; + ifp->speed = ifm->ifm_data.ifi_baudrate / 1000000; - if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("%s: interface %s index %d", __func__, ifp->name, - ifp->ifindex); + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: interface %s index %d", __func__, + ifp->name, ifp->ifindex); + } return 0; } -- cgit v1.2.3