proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-08-05 06:34:44 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

bgpd: modify attr fields before hash insert

Browse Source 
bgp_attr_intern(attr) takes an attribute, duplicates it, and inserts it
into the attribute hash table, returning the inserted attr. This is done
when processing a bgp update. We store the returned attribute in the
path info struct. However, later on we modify one of the fields of the
attribute. This field is inspected by attrhash_cmp, the function that
allows the hash table to select the correct item from the hash chain for
a given key when doing a lookup on an item. By modifying the field after
it's been inserted, we open the possibility that two items in the same
chain that at insertion time were differential by attrhash_cmp becomes
equal according to that function. When performing subsequent hash
lookups, it is then indeterminate which of the equivalent items the hash
table will select from the chain (in practice it is the first one but
this may not be the one we want). Thus, it is illegal to modify
data used by a hash comparison function after inserting that data into
a hash table.

In fact this is occurring for attributes. We insert two attributes that
hash to the same key and thus end up in the same hash chain. Then we
modify one of them such that the two items now compare equal. Later one
we want to release the second item from the chain before XFREE()'ing it,
but since the two items compare equal we get the first item back, then
free the second one, which constitutes two bugs, the first being the
wrong attribute removed from the hash table and the second being a
dangling pointer stored in the hash table.

To rectify this we need to perform any modifications to an attr before
it is inserted into the table, i.e., before calling bgp_attr_intern().
This patch does that by moving the sole modification to the attr that
occurs after the insert (that I have seen) before that call.

Signed-off-by: Quentin Young <qlyoung@nvidia.com>
This commit is contained in:
Quentin Young 2020-09-02 13:16:35 -04:00
parent 2e5b029865
commit 1e9be514b3
1 changed files with 6 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
bgpd/bgp_route.c
View File

@ -3605,6 +3605,12 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
goto filtered;
}
/* Update Overlay Index */
if (afi == AFI_L2VPN) {
overlay_index_update(&new_attr,
evpn == NULL ? NULL : &evpn->gw_ip);
}
attr_new = bgp_attr_intern(&new_attr);
/* If maximum prefix count is configured and current prefix
@ -3850,12 +3856,6 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
}
}
#endif
/* Update Overlay Index */
if (afi == AFI_L2VPN) {
overlay_index_update(
pi->attr,
evpn == NULL ? NULL : &evpn->gw_ip);
}
/* Update bgp route dampening information. */
if (CHECK_FLAG(bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)