From 31d39e72214440b781f70d549e4c9313bc23c46e Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 12 Sep 2024 09:31:49 +0200 Subject: [PATCH] isisd: fix rcap tlv double-free crash A double-free crash happens when a subTLV of the "Router Capability" TLV is not readable and a previous "Router Capability" TLV was read. rcap was supposed to be freed later by isis_free_tlvs() -> free_tlv_router_cap(). In 78774bbcd5 ("isisd: add isis flex-algo lsp advertisement"), this was not the case because rcap was not saved to tlvs->router_cap when the function returned early because of a subTLV length issue. Always set tlvs->router_cap to free the memory. Note that this patch has the consequence that in case of subTLV error, the previously read "Router Capability" subTLVs are kept in memory. Fixes: 49efc80d34 ("isisd: Ensure rcap is freed in error case") Fixes: 78774bbcd5 ("isisd: add isis flex-algo lsp advertisement") Reported-by: Iggy Frankovic Signed-off-by: Louis Scalbert (cherry picked from commit d61758140d33972c10ecbb72d0a3e528049dd8d6) --- isisd/isis_tlvs.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c index 59d632d170ed..fe0bf4ae3e09 100644 --- a/isisd/isis_tlvs.c +++ b/isisd/isis_tlvs.c @@ -5270,16 +5270,17 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, return 0; } - if (tlvs->router_cap) - /* Multiple Router Capability found */ - rcap = tlvs->router_cap; - else { - /* Allocate router cap structure and initialize SR Algorithms */ - rcap = XCALLOC(MTYPE_ISIS_TLV, sizeof(struct isis_router_cap)); + if (!tlvs->router_cap) { + /* First Router Capability TLV. + * Allocate router cap structure and initialize SR Algorithms */ + tlvs->router_cap = XCALLOC(MTYPE_ISIS_TLV, + sizeof(struct isis_router_cap)); for (int i = 0; i < SR_ALGORITHM_COUNT; i++) - rcap->algo[i] = SR_ALGORITHM_UNSET; + tlvs->router_cap->algo[i] = SR_ALGORITHM_UNSET; } + rcap = tlvs->router_cap; + /* Get Router ID and Flags */ rcap->router_id.s_addr = stream_get_ipv4(s); rcap->flags = stream_getc(s); @@ -5301,7 +5302,6 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, log, indent, "WARNING: Router Capability subTLV length too large compared to expected size\n"); stream_forward_getp(s, STREAM_READABLE(s)); - XFREE(MTYPE_ISIS_TLV, rcap); return 0; } @@ -5612,7 +5612,6 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, } subtlv_len = subtlv_len - length - 2; } - tlvs->router_cap = rcap; return 0; }