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 <sworley@cumulusnetworks.com>
This commit is contained in:
Stephen Worley 2019-10-15 15:39:49 -04:00
parent b77a69bdc6
commit b19d55d048
4 changed files with 20 additions and 19 deletions

View File

@ -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,

View File

@ -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,
struct interface *ifp;
ifp = if_lookup_by_index_per_ns(zvrf->zns,
zpr.rule.ifindex);
if (!zpr.ifp) {
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))

View File

@ -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)

View File

@ -41,7 +41,7 @@ struct zebra_pbr_rule {
struct pbr_rule rule;
struct interface *ifp;
char ifname[INTERFACE_NAMSIZ];
vrf_id_t vrf_id;
};