summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorQuentin Young <qlyoung@cumulusnetworks.com>2020-03-04 02:27:49 +0100
committerQuentin Young <qlyoung@cumulusnetworks.com>2020-03-05 19:27:01 +0100
commit61980c71c435f19142b32fdb7bf7d214a534684c (patch)
tree7d1015215382f142ba9b4e4d7ab0300253f157cf
parentMerge pull request #5856 from pguibert6WIND/nhrp_override_fix (diff)
downloadfrr-61980c71c435f19142b32fdb7bf7d214a534684c.tar.xz
frr-61980c71c435f19142b32fdb7bf7d214a534684c.zip
vrrpd: always null mvl_ifp ptr when mvl is deleted
When we get a deletion notification for the macvlan device, we need to do two things. First, down the VRRP session if it's up. Second, since the mvl device is dynamic (i.e. not explicitly configured by FRR) it will be deleted upon return from the callback, so we need to drop the pointer to it. The checks for the first and second one were one check so the pointer was only nulled when the session was already up, leading to a later heap UAF on the mvl ifp. Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
-rw-r--r--vrrpd/vrrp.c48
1 files changed, 40 insertions, 8 deletions
diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c
index da6de341e..431ebf662 100644
--- a/vrrpd/vrrp.c
+++ b/vrrpd/vrrp.c
@@ -2219,15 +2219,47 @@ void vrrp_if_del(struct interface *ifp)
vrrp_if_down(ifp);
for (ALL_LIST_ELEMENTS_RO(vrs, ln, vr)) {
- if ((vr->v4->mvl_ifp == ifp || vr->ifp == ifp)
- && vr->v4->fsm.state != VRRP_STATE_INITIALIZE) {
- vrrp_event(vr->v4, VRRP_EVENT_SHUTDOWN);
- vr->v4->mvl_ifp = NULL;
- } else if ((vr->v6->mvl_ifp == ifp || vr->ifp == ifp)
- && vr->v6->fsm.state != VRRP_STATE_INITIALIZE) {
- vrrp_event(vr->v6, VRRP_EVENT_SHUTDOWN);
- vr->v6->mvl_ifp = NULL;
+ if (vr->v4->mvl_ifp == ifp || vr->ifp == ifp) {
+ /*
+ * If either of our interfaces goes away, we need to
+ * down the session
+ */
+ if (vr->v4->fsm.state != VRRP_STATE_INITIALIZE)
+ vrrp_event(vr->v4, VRRP_EVENT_SHUTDOWN);
+
+ /*
+ * If it was the macvlan, then it wasn't explicitly
+ * configured and will be deleted when we return from
+ * this function, so we need to lose the reference
+ */
+ if (ifp == vr->v4->mvl_ifp)
+ vr->v4->mvl_ifp = NULL;
+
+ } else if (vr->v6->mvl_ifp == ifp || vr->ifp == ifp) {
+ /*
+ * If either of our interfaces goes away, we need to
+ * down the session
+ */
+ if (vr->v6->fsm.state != VRRP_STATE_INITIALIZE)
+ vrrp_event(vr->v6, VRRP_EVENT_SHUTDOWN);
+
+ /*
+ * If it was the macvlan, then it wasn't explicitly
+ * configured and will be deleted when we return from
+ * this function, so we need to lose the reference
+ */
+ if (ifp == vr->v6->mvl_ifp)
+ vr->v6->mvl_ifp = NULL;
}
+
+ /*
+ * We shouldn't need to lose the reference if it's the primary
+ * interface, because that was configured explicitly in our
+ * config, and thus will be kept as a stub; to avoid stupid
+ * bugs, double check that
+ */
+ if (ifp == vr->ifp)
+ assert(ifp->configured);
}
list_delete(&vrs);