bgpd: Ensure that the correct aspath is free'd

donaldsharp · donaldsharp · commit e613e12f12b9 · 2024-03-13T19:28:11.000-04:00
Currently in subgroup_default_originate the attr.aspath
is set in bgp_attr_default_set, which hashs the aspath
and creates a refcount for it.  If this is a withdraw
the subgroup_announce_check and bgp_adj_out_set_subgroup
is called which will intern the attribute.  This will
cause the the attr.aspath to be set to a new value
finally at the bottom of the function it intentionally
uninterns the aspath which is not the one that was
created for this function.  This reduces the other
aspath's refcount by 1 and if a clear bgp * is issued
fast enough the aspath for that will be removed
and the system will crash.

Signed-off-by: Donald Sharp &lt;sharpd@nvidia.com&gt;
diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c
@@ -854,6 +854,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw)
 	struct bgp *bgp;
 	struct attr attr;
 	struct attr *new_attr = &attr;
+	struct aspath *aspath;
 	struct prefix p;
 	struct peer *from;
 	struct bgp_dest *dest;
@@ -891,6 +892,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw)
 	/* make coverity happy */
 	assert(attr.aspath);
 
+	aspath = attr.aspath;
 	attr.med = 0;
 	attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC);
 
@@ -1046,7 +1048,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw)
 		}
 	}
 
-	aspath_unintern(&attr.aspath);
+	aspath_unintern(&aspath);
 }
 
 /*

Original file line number	Diff line number	Diff line change
`@@ -854,6 +854,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw)`
`854`	`854`	`struct bgp *bgp;`
`855`	`855`	`struct attr attr;`
`856`	`856`	`struct attr *new_attr = &attr;`
	`857`	`+ struct aspath *aspath;`
`857`	`858`	`struct prefix p;`
`858`	`859`	`struct peer *from;`
`859`	`860`	`struct bgp_dest *dest;`
`@@ -891,6 +892,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw)`
`891`	`892`	`/* make coverity happy */`
`892`	`893`	`assert(attr.aspath);`
`893`	`894`
	`895`	`+ aspath = attr.aspath;`
`894`	`896`	`attr.med = 0;`
`895`	`897`	`attr.flag \|= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC);`
`896`	`898`
`@@ -1046,7 +1048,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw)`
`1046`	`1048`	`}`
`1047`	`1049`	`}`
`1048`	`1050`
`1049`		`- aspath_unintern(&attr.aspath);`
	`1051`	`+ aspath_unintern(&aspath);`
`1050`	`1052`	`}`
`1051`	`1053`
`1052`	`1054`	`/*`