From b19d55d048f6a644629595da361df9e6d0674555 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Oct 2019 15:39:49 -0400 Subject: [PATCH] zebra: Don't bother ref'ing ifp in zebra_pbr_rule If we only really use the ifp for the name, then don't bother referencing the ifp. If that ifp is freed, we don't expect zebra to handle the rules that use it (that's pbrd's job), so it is going to be pointing to unintialized memory when we decide to remove that rule later. Thus, just keep the name in the data and dont mess with pointer refs. Signed-off-by: Stephen Worley --- zebra/rule_netlink.c | 20 ++++++++++---------- zebra/zapi_msg.c | 15 ++++++++------- zebra/zebra_pbr.c | 2 +- zebra/zebra_pbr.h | 2 +- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/zebra/rule_netlink.c b/zebra/rule_netlink.c index 1b465b0335..2fdb215128 100644 --- a/zebra/rule_netlink.c +++ b/zebra/rule_netlink.c @@ -86,9 +86,8 @@ static int netlink_rule_update(int cmd, struct zebra_pbr_rule *rule) addattr32(&req.n, sizeof(req), FRA_PRIORITY, rule->rule.priority); /* interface on which applied */ - if (rule->ifp) - addattr_l(&req.n, sizeof(req), FRA_IFNAME, rule->ifp->name, - strlen(rule->ifp->name) + 1); + addattr_l(&req.n, sizeof(req), FRA_IFNAME, rule->ifname, + strlen(rule->ifname) + 1); /* source IP, if specified */ if (IS_RULE_FILTERING_ON_SRC_IP(rule)) { @@ -122,8 +121,7 @@ static int netlink_rule_update(int cmd, struct zebra_pbr_rule *rule) zlog_debug( "Tx %s family %s IF %s(%u) Pref %u Fwmark %u Src %s Dst %s Table %u", nl_msg_type_to_str(cmd), nl_family_to_str(family), - rule->ifp ? rule->ifp->name : "Unknown", - rule->rule.ifindex, rule->rule.priority, + rule->ifname, rule->rule.ifindex, rule->rule.priority, rule->rule.filter.fwmark, prefix2str(&rule->rule.filter.src_ip, buf1, sizeof(buf1)), @@ -227,13 +225,15 @@ int netlink_rule_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (tb[FRA_IFNAME] == NULL) return 0; - /* If we don't know the interface, we don't care. */ ifname = (char *)RTA_DATA(tb[FRA_IFNAME]); zns = zebra_ns_lookup(ns_id); - rule.ifp = if_lookup_by_name_per_ns(zns, ifname); - if (!rule.ifp) + + /* If we don't know the interface, we don't care. */ + if (!if_lookup_by_name_per_ns(zns, ifname)) return 0; + strlcpy(rule.ifname, ifname, sizeof(rule.ifname)); + if (tb[FRA_PRIORITY]) rule.rule.priority = *(uint32_t *)RTA_DATA(tb[FRA_PRIORITY]); @@ -268,8 +268,8 @@ int netlink_rule_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) zlog_debug( "Rx %s family %s IF %s(%u) Pref %u Src %s Dst %s Table %u", nl_msg_type_to_str(h->nlmsg_type), - nl_family_to_str(frh->family), rule.ifp->name, - rule.ifp->ifindex, rule.rule.priority, + nl_family_to_str(frh->family), rule.ifname, + rule.rule.ifindex, rule.rule.priority, prefix2str(&rule.rule.filter.src_ip, buf1, sizeof(buf1)), prefix2str(&rule.rule.filter.dst_ip, buf2, diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 96faf6f1f7..e61e68b7fe 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -783,10 +783,7 @@ void zsend_rule_notify_owner(struct zebra_pbr_rule *rule, stream_putl(s, rule->rule.seq); stream_putl(s, rule->rule.priority); stream_putl(s, rule->rule.unique); - if (rule->ifp) - stream_putl(s, rule->ifp->ifindex); - else - stream_putl(s, 0); + stream_putl(s, rule->rule.ifindex); stream_putw_at(s, 0, stream_get_endp(s)); @@ -2321,13 +2318,17 @@ static inline void zread_rule(ZAPI_HANDLER_ARGS) STREAM_GETL(s, zpr.rule.ifindex); if (zpr.rule.ifindex) { - zpr.ifp = if_lookup_by_index_per_ns(zvrf->zns, - zpr.rule.ifindex); - if (!zpr.ifp) { + struct interface *ifp; + + ifp = if_lookup_by_index_per_ns(zvrf->zns, + zpr.rule.ifindex); + if (!ifp) { zlog_debug("Failed to lookup ifindex: %u", zpr.rule.ifindex); return; } + + strlcpy(zpr.ifname, ifp->name, sizeof(zpr.ifname)); } if (!is_default_prefix(&zpr.rule.filter.src_ip)) diff --git a/zebra/zebra_pbr.c b/zebra/zebra_pbr.c index e77e29f13a..e24d2e2b42 100644 --- a/zebra/zebra_pbr.c +++ b/zebra/zebra_pbr.c @@ -191,7 +191,7 @@ bool zebra_pbr_rules_hash_equal(const void *arg1, const void *arg2) if (!prefix_same(&r1->rule.filter.dst_ip, &r2->rule.filter.dst_ip)) return false; - if (r1->ifp != r2->ifp) + if (r1->rule.ifindex != r2->rule.ifindex) return false; if (r1->vrf_id != r2->vrf_id) diff --git a/zebra/zebra_pbr.h b/zebra/zebra_pbr.h index fcc9c5c39a..b7fbc9b7d5 100644 --- a/zebra/zebra_pbr.h +++ b/zebra/zebra_pbr.h @@ -41,7 +41,7 @@ struct zebra_pbr_rule { struct pbr_rule rule; - struct interface *ifp; + char ifname[INTERFACE_NAMSIZ]; vrf_id_t vrf_id; };