Skip to content

Commit

Permalink
zebra: Don't bother ref'ing ifp in zebra_pbr_rule
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
sworleys committed Oct 15, 2019
1 parent b77a69b commit b19d55d
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 19 deletions.
20 changes: 10 additions & 10 deletions zebra/rule_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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]);

Expand Down Expand Up @@ -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,
Expand Down
15 changes: 8 additions & 7 deletions zebra/zapi_msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion zebra/zebra_pbr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion zebra/zebra_pbr.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ struct zebra_pbr_rule {

struct pbr_rule rule;

struct interface *ifp;
char ifname[INTERFACE_NAMSIZ];

vrf_id_t vrf_id;
};
Expand Down

0 comments on commit b19d55d

Please sign in to comment.