Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
From ce67e8fd10f0250c187bab529808457c74fc4b0c Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Thu, 24 Mar 2022 20:02:33 -0400
Subject: [PATCH] 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)
---
zebra/if_netlink.c | 4 ++--
zebra/interface.c | 5 +++--
zebra/interface.h | 2 +-
zebra/kernel_socket.c | 27 +++++++++++++++------------
zebra/zebra_netns_notify.c | 2 +-
5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index 141e4074d56..5a2f6d33b18 100644
--- 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 e4e80ec4e93..cb5dc83685c 100644
--- 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 771398b5476..571831f87da 100644
--- 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 573db4ae5d7..d6c43dbcb06 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;
}
diff --git a/zebra/zebra_netns_notify.c b/zebra/zebra_netns_notify.c
index 8cc7724f053..155079cfb41 100644
--- 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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
From b0c68c92bb256a4d47644bcd734f9dc9e5f53567 Mon Sep 17 00:00:00 2001
From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 26 Apr 2023 23:25:27 -0400
Subject: [PATCH] zebra: Rename vrf_lookup_by_tableid to zebra_vrf_lookup..

Rename the vrf_lookup_by_id function to zebra_vrf_lookup_by_id
and move to zebra_vrf.c where it nominally belongs, as that
we need zebra specific data to find this vrf_id and as such
it does not belong in vrf.c

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---
zebra/if_netlink.c | 3 ++-
zebra/rt_netlink.c | 31 ++-----------------------------
zebra/rt_netlink.h | 1 -
zebra/zebra_vrf.c | 27 +++++++++++++++++++++++++++
zebra/zebra_vrf.h | 1 +
5 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c
index 5a2f6d33b18..b3d74a2e462 100644
--- a/zebra/if_netlink.c
+++ b/zebra/if_netlink.c
@@ -339,7 +339,8 @@ static void netlink_vrf_change(struct nlmsghdr *h, struct rtattr *tb,
if (!vrf_lookup_by_id((vrf_id_t)ifi->ifi_index)) {
vrf_id_t exist_id;

- exist_id = vrf_lookup_by_table(nl_table_id, ns_id);
+ exist_id =
+ zebra_vrf_lookup_by_table(nl_table_id, ns_id);
if (exist_id != VRF_DEFAULT) {
vrf = vrf_lookup_by_id(exist_id);

diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index 24c01b7f518..e4c1393bd19 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -361,33 +361,6 @@ static inline int proto2zebra(int proto, int family, bool is_nexthop)
return proto;
}

-/*
-Pending: create an efficient table_id (in a tree/hash) based lookup)
- */
-vrf_id_t vrf_lookup_by_table(uint32_t table_id, ns_id_t ns_id)
-{
- struct vrf *vrf;
- struct zebra_vrf *zvrf;
-
- RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) {
- zvrf = vrf->info;
- if (zvrf == NULL)
- continue;
- /* case vrf with netns : match the netnsid */
- if (vrf_is_backend_netns()) {
- if (ns_id == zvrf_id(zvrf))
- return zvrf_id(zvrf);
- } else {
- /* VRF is VRF_BACKEND_VRF_LITE */
- if (zvrf->table_id != table_id)
- continue;
- return zvrf_id(zvrf);
- }
- }
-
- return VRF_DEFAULT;
-}
-
/**
* @parse_encap_mpls() - Parses encapsulated mpls attributes
* @tb: Pointer to rtattr to look for nested items in.
@@ -761,7 +734,7 @@ static int netlink_route_change_read_unicast(struct nlmsghdr *h, ns_id_t ns_id,
table = rtm->rtm_table;

/* Map to VRF */
- vrf_id = vrf_lookup_by_table(table, ns_id);
+ vrf_id = zebra_vrf_lookup_by_table(table, ns_id);
if (vrf_id == VRF_DEFAULT) {
if (!is_zebra_valid_kernel_table(table)
&& !is_zebra_main_routing_table(table))
@@ -1032,7 +1005,7 @@ static int netlink_route_change_read_multicast(struct nlmsghdr *h,
else
table = rtm->rtm_table;

- vrf = vrf_lookup_by_table(table, ns_id);
+ vrf = zebra_vrf_lookup_by_table(table, ns_id);

if (tb[RTA_IIF])
iif = *(int *)RTA_DATA(tb[RTA_IIF]);
diff --git a/zebra/rt_netlink.h b/zebra/rt_netlink.h
index 93c06e555b0..51c13574c70 100644
--- a/zebra/rt_netlink.h
+++ b/zebra/rt_netlink.h
@@ -103,7 +103,6 @@ extern int netlink_macfdb_read_specific_mac(struct zebra_ns *zns,
uint16_t vid);
extern int netlink_neigh_read_specific_ip(const struct ipaddr *ip,
struct interface *vlan_if);
-extern vrf_id_t vrf_lookup_by_table(uint32_t table_id, ns_id_t ns_id);

struct nl_batch;
extern enum netlink_msg_status
diff --git a/zebra/zebra_vrf.c b/zebra/zebra_vrf.c
index f88a65d9520..5fee5a2db91 100644
--- a/zebra/zebra_vrf.c
+++ b/zebra/zebra_vrf.c
@@ -393,6 +393,33 @@ struct zebra_vrf *zebra_vrf_alloc(struct vrf *vrf)
return zvrf;
}

+/*
+Pending: create an efficient table_id (in a tree/hash) based lookup)
+ */
+vrf_id_t zebra_vrf_lookup_by_table(uint32_t table_id, ns_id_t ns_id)
+{
+ struct vrf *vrf;
+ struct zebra_vrf *zvrf;
+
+ RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) {
+ zvrf = vrf->info;
+ if (zvrf == NULL)
+ continue;
+ /* case vrf with netns : match the netnsid */
+ if (vrf_is_backend_netns()) {
+ if (ns_id == zvrf_id(zvrf))
+ return zvrf_id(zvrf);
+ } else {
+ /* VRF is VRF_BACKEND_VRF_LITE */
+ if (zvrf->table_id != table_id)
+ continue;
+ return zvrf_id(zvrf);
+ }
+ }
+
+ return VRF_DEFAULT;
+}
+
/* Lookup VRF by identifier. */
struct zebra_vrf *zebra_vrf_lookup_by_id(vrf_id_t vrf_id)
{
diff --git a/zebra/zebra_vrf.h b/zebra/zebra_vrf.h
index a24a008b761..031b80ebb0f 100644
--- a/zebra/zebra_vrf.h
+++ b/zebra/zebra_vrf.h
@@ -257,6 +257,7 @@ extern struct route_table *zebra_vrf_get_table_with_table_id(afi_t afi,
extern void zebra_vrf_update_all(struct zserv *client);
extern struct zebra_vrf *zebra_vrf_lookup_by_id(vrf_id_t vrf_id);
extern struct zebra_vrf *zebra_vrf_lookup_by_name(const char *);
+extern vrf_id_t zebra_vrf_lookup_by_table(uint32_t table_id, ns_id_t ns_id);
extern struct zebra_vrf *zebra_vrf_alloc(struct vrf *vrf);
extern struct route_table *zebra_vrf_table(afi_t, safi_t, vrf_id_t);

Loading