Fix bgp transit double free (#5436)

Fix bgp transit double free
This commit is contained in:
David Lamparter 2019-12-10 17:56:57 +01:00 committed by GitHub
commit 2d7932e153
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 78 additions and 66 deletions

View File

@ -421,14 +421,15 @@ static struct transit *transit_intern(struct transit *transit)
return find;
}
void transit_unintern(struct transit *transit)
static void transit_unintern(struct transit **transit)
{
if (transit->refcnt)
transit->refcnt--;
if ((*transit)->refcnt)
(*transit)->refcnt--;
if (transit->refcnt == 0) {
hash_release(transit_hash, transit);
transit_free(transit);
if ((*transit)->refcnt == 0) {
hash_release(transit_hash, *transit);
transit_free(*transit);
*transit = NULL;
}
}
@ -851,7 +852,7 @@ void bgp_attr_unintern_sub(struct attr *attr)
UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_CLUSTER_LIST));
if (attr->transit)
transit_unintern(attr->transit);
transit_unintern(&attr->transit);
if (attr->encap_subtlvs)
encap_unintern(&attr->encap_subtlvs, ENCAP_SUBTLV_TYPE);
@ -2483,7 +2484,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
/* Fetch attribute flag and type. */
@ -2506,7 +2508,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
/* Check extended attribue length bit. */
@ -2527,7 +2530,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR);
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
/* Set type to bitmap to check duplicate attribute. `type' is
@ -2584,7 +2588,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata,
ndl + lfl + 1);
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
struct bgp_attr_parser_args attr_args = {
@ -2609,7 +2614,7 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
attr_args.total);
if (ret == BGP_ATTR_PARSE_PROCEED)
continue;
return ret;
goto done;
}
/* OK check attribute and store it's value. */
@ -2687,32 +2692,25 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR);
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
if (ret == BGP_ATTR_PARSE_EOR) {
if (as4_path)
aspath_unintern(&as4_path);
return ret;
goto done;
}
/* If hard error occurred immediately return to the caller. */
if (ret == BGP_ATTR_PARSE_ERROR) {
flog_warn(EC_BGP_ATTRIBUTE_PARSE_ERROR,
"%s: Attribute %s, parse error", peer->host,
lookup_msg(attr_str, type, NULL));
if (as4_path)
aspath_unintern(&as4_path);
return ret;
goto done;
}
if (ret == BGP_ATTR_PARSE_WITHDRAW) {
flog_warn(
EC_BGP_ATTRIBUTE_PARSE_WITHDRAW,
"%s: Attribute %s, parse error - treating as withdrawal",
peer->host, lookup_msg(attr_str, type, NULL));
if (as4_path)
aspath_unintern(&as4_path);
return ret;
goto done;
}
/* Check the fetched length. */
@ -2722,9 +2720,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
peer->host, lookup_msg(attr_str, type, NULL));
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
if (as4_path)
aspath_unintern(&as4_path);
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
}
@ -2735,9 +2732,9 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
lookup_msg(attr_str, type, NULL));
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
if (as4_path)
aspath_unintern(&as4_path);
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
/*
@ -2756,16 +2753,14 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP))
&& !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) {
if (bgp_attr_nexthop_valid(peer, attr) < 0) {
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
}
/* Check all mandatory well-known attributes are present */
if ((ret = bgp_attr_check(peer, attr)) < 0) {
if (as4_path)
aspath_unintern(&as4_path);
return ret;
}
if ((ret = bgp_attr_check(peer, attr)) < 0)
goto done;
/*
* At this place we can see whether we got AS4_PATH and/or
@ -2788,28 +2783,10 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
&as4_aggregator_addr)) {
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR);
if (as4_path)
aspath_unintern(&as4_path);
return BGP_ATTR_PARSE_ERROR;
ret = BGP_ATTR_PARSE_ERROR;
goto done;
}
/* At this stage, we have done all fiddling with as4, and the
* resulting info is in attr->aggregator resp. attr->aspath
* so we can chuck as4_aggregator and as4_path alltogether in
* order to save memory
*/
if (as4_path) {
aspath_unintern(&as4_path); /* unintern - it is in the hash */
/* The flag that we got this is still there, but that does not
* do any trouble
*/
}
/*
* The "rest" of the code does nothing with as4_aggregator.
* there is no memory attached specifically which is not part
* of the attr.
* so ignoring just means do nothing.
*/
/*
* Finally do the checks on the aspath we did not do yet
* because we waited for a potentially synthesized aspath.
@ -2817,21 +2794,59 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH))) {
ret = bgp_attr_aspath_check(peer, attr);
if (ret != BGP_ATTR_PARSE_PROCEED)
return ret;
goto done;
}
/* Finally intern unknown attribute. */
ret = BGP_ATTR_PARSE_PROCEED;
done:
/*
* At this stage, we have done all fiddling with as4, and the
* resulting info is in attr->aggregator resp. attr->aspath so
* we can chuck as4_aggregator and as4_path alltogether in order
* to save memory
*/
if (as4_path) {
/*
* unintern - it is in the hash
* The flag that we got this is still there, but that
* does not do any trouble
*/
aspath_unintern(&as4_path);
}
if (ret != BGP_ATTR_PARSE_ERROR) {
/* Finally intern unknown attribute. */
if (attr->transit)
attr->transit = transit_intern(attr->transit);
if (attr->encap_subtlvs)
attr->encap_subtlvs = encap_intern(attr->encap_subtlvs,
ENCAP_SUBTLV_TYPE);
#if ENABLE_BGP_VNC
if (attr->vnc_subtlvs)
attr->vnc_subtlvs = encap_intern(attr->vnc_subtlvs,
VNC_SUBTLV_TYPE);
#endif
} else {
if (attr->transit) {
transit_free(attr->transit);
attr->transit = NULL;
}
bgp_attr_flush_encap(attr);
};
/* Sanity checks */
if (attr->transit)
attr->transit = transit_intern(attr->transit);
assert(attr->transit->refcnt > 0);
if (attr->encap_subtlvs)
attr->encap_subtlvs =
encap_intern(attr->encap_subtlvs, ENCAP_SUBTLV_TYPE);
assert(attr->encap_subtlvs->refcnt > 0);
#if ENABLE_BGP_VNC
if (attr->vnc_subtlvs)
attr->vnc_subtlvs =
encap_intern(attr->vnc_subtlvs, VNC_SUBTLV_TYPE);
assert(attr->vnc_subtlvs->refcnt > 0);
#endif
return BGP_ATTR_PARSE_PROCEED;
return ret;
}
/*

View File

@ -304,9 +304,6 @@ extern unsigned long int attr_unknown_count(void);
extern int cluster_loop_check(struct cluster_list *, struct in_addr);
extern void cluster_unintern(struct cluster_list *);
/* Transit attribute prototypes. */
void transit_unintern(struct transit *);
/* Below exported for unit-test purposes only */
struct bgp_attr_parser_args {
struct peer *peer;