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>
(cherry picked from commit d0438da6b09333d2b77a9eac2e9fffbbae6e603b)

diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index 141e4074d56be43cf4b489ea3be4155812a2ead1..5a2f6d33b18cd6085c02087fb3e4639c2b721dbb 100644 (file)
--- a/zebra/if_netlink.c
+++ b/zebra/if_netlink.c
@@ -101,7 +101,7 @@ static void set_ifindex(struct interface *ifp, ifindex_t ifi_index,
                                        EC_LIB_INTERFACE,
                                        "interface rename detected on up interface: index %d was renamed from %s to %s, results are uncertain!",
                                        ifi_index, oifp->name, ifp->name);
-                       if_delete_update(oifp);
+                       if_delete_update(&oifp);
                }
        }
        if_set_index(ifp, ifi_index);
@@ -2031,7 +2031,7 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup)
                else if (IS_ZEBRA_IF_VXLAN(ifp))
                        zebra_l2_vxlanif_del(ifp);
 
-               if_delete_update(ifp);
+               if_delete_update(&ifp);
 
                /* If VRF, delete the VRF structure itself. */
                if (zif_type == ZEBRA_IF_VRF && !vrf_is_backend_netns())

diff --git a/zebra/interface.c b/zebra/interface.c
index e4e80ec4e93e943b332256242deda2ceb53af729..cb5dc83685ccf6c57236824bc10f8c467f6d34c3 100644 (file)
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -769,9 +769,10 @@ static void if_delete_connected(struct interface *ifp)
 }
 
 /* Handle an interface delete event */
-void if_delete_update(struct interface *ifp)
+void if_delete_update(struct interface **pifp)
 {
        struct zebra_if *zif;
+       struct interface *ifp = *pifp;
 
        if (if_is_up(ifp)) {
                flog_err(
@@ -834,7 +835,7 @@ void if_delete_update(struct interface *ifp)
                if (IS_ZEBRA_DEBUG_KERNEL)
                        zlog_debug("interface %s is being deleted from the system",
                                   ifp->name);
-               if_delete(&ifp);
+               if_delete(pifp);
        }
 }
 

diff --git a/zebra/interface.h b/zebra/interface.h
index 771398b54762fae9ef5697484fb009232bd33fee..571831f87dabd6fa92a1ea69f449f1bae75a88f5 100644 (file)
--- a/zebra/interface.h
+++ b/zebra/interface.h
@@ -474,7 +474,7 @@ extern void if_nbr_ipv6ll_to_ipv4ll_neigh_update(struct interface *ifp,
                                                 struct in6_addr *address,
                                                 int add);
 extern void if_nbr_ipv6ll_to_ipv4ll_neigh_del_all(struct interface *ifp);
-extern void if_delete_update(struct interface *ifp);
+extern void if_delete_update(struct interface **ifp);
 extern void if_add_update(struct interface *ifp);
 extern void if_up(struct interface *);
 extern void if_down(struct interface *);

diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c
index 573db4ae5d7321915224a4759abcbdd851874d60..d6c43dbcb069eeffc7bf64bbb3d7906b82211635 100644 (file)
--- 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;
 }

diff --git a/zebra/zebra_netns_notify.c b/zebra/zebra_netns_notify.c
index 8cc7724f053891bd9a3972e649c6fdf0bb594191..155079cfb410ed5c8c955a1960557fcb7d70842f 100644 (file)
--- a/zebra/zebra_netns_notify.c
+++ b/zebra/zebra_netns_notify.c
@@ -179,7 +179,7 @@ static int zebra_ns_delete(char *name)
                }
 
                UNSET_FLAG(ifp->flags, IFF_UP);
-               if_delete_update(ifp);
+               if_delete_update(&ifp);
        }
 
        ns = (struct ns *)vrf->ns_ctxt;