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

1 files changed, 15 insertions, 12 deletions

diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c
index 6583af0a5..0d2830a39 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;
 }