Commit 887db2c408 for frr
commit 887db2c408b7635c3c573a9fff76af40ad6ffd87
Author: Philippe Guibert <philippe.guibert@6wind.com>
Date: Mon Oct 26 18:17:55 2026 +0100
isisd: fix router capability TLV length overflow
When many flex-algo definitions are advertised, the length of the Router
Capability TLV (242) can exceed 255 bytes and wrap. For instance, with
14 flex-algos that each advertise an exclude-any and an include-any
affinity, each FAD sub-TLV takes 18 bytes:
> affinity-map red bit-position 0
> affinity-map blue bit-position 1
> router isis 1
> flex-algo 128
> advertise-definition
> affinity exclude-any red
> affinity include-any blue
> [..]
> flex-algo 141
> advertise-definition
> affinity exclude-any red
> affinity include-any blue
Without segment-routing sub-TLVs, the first 13 FADs fill the TLV up to
239 bytes (5 + 13 * 18). The 14th one is added to the same TLV, which
ends at 257 bytes, and its length field is set to 1. The neighbor
ignores the Router Capability TLV, and none of the flex-algo definitions
is known:
> r2# show isis database detail r1.00-00
> Area 1:
> IS-IS Level-2 link-state database:
> LSP ID PduLen SeqNumber Chksum Holdtime ATT/P/OL
> 0000.0000.0001.00-00 343 0x00000002 0xa7d1 1112 0/0/0
> Hostname: r1
> Protocols Supported: IPv4
> Area Address: 49.0001
> TE Router ID: 192.0.2.1
> Extended Reachability: 0000.0000.0002.00 (Metric: 10)
> IPv4 Interface Address: 192.0.2.1
> Extended IP Reachability: 192.0.2.1/32 (Metric: 10)
> Extended IP Reachability: 10.0.1.0/24 (Metric: 10)
Before adding a FAD sub-TLV, pack_tlv_router_cap() only starts a new
Router Capability TLV when the current one already exceeds 250 bytes,
regardless of the size of the FAD sub-TLV to add. The SRv6 capability,
algorithm and MSD sub-TLVs, added after the FADs, are not checked at
all. isis_router_cap_tlv_size() uses another criterion (a wrap of the
cumulated size modulo 256) to estimate the stream space needed.
Fix this by starting a new Router Capability TLV when the sub-TLV to
add does not fit in the current one, for the FAD and the SRv6 sub-TLVs,
and use the same calculation in isis_router_cap_tlv_size(). The latter
also counts the SRv6 MSD entries instead of summing their values, and
does not count the SR algorithm sub-TLV twice, as done when packing.
The 14th FAD is now carried by a second Router Capability TLV, and the
neighbor decodes the 14 definitions:
> r2# show isis database detail r1.00-00
> [..]
> 0000.0000.0001.00-00 350 0x00000002 0x5666 1186 0/0/0
> Hostname: r1
> Protocols Supported: IPv4
> Area Address: 49.0001
> TE Router ID: 192.0.2.1
> Router Capability: 192.0.2.1 , D:0, S:0
> Flex-Algo Definition: 128
> Metric-Type: 0
> Calc-Type: 0
> Priority: 128
> Exclude-Any: 0x00000001
> Include-Any: 0x00000002
> Include-All: not-set
> M-Flag: 0
> [..]
> Flex-Algo Definition: 141
> Metric-Type: 0
> Calc-Type: 0
> Priority: 128
> Exclude-Any: 0x00000001
> Include-Any: 0x00000002
> Include-All: not-set
> M-Flag: 0
> Extended Reachability: 0000.0000.0002.00 (Metric: 10)
> [..]
Fixes: 78774bbcd51b ("isisd: add isis flex-algo lsp advertisement")
Fixes: 451ef283ccee ("isisd: Pack SRv6 Capabilities Sub-TLV")
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c
index d24c55feb8..5898f57aee 100644
--- a/isisd/isis_tlvs.c
+++ b/isisd/isis_tlvs.c
@@ -4909,67 +4909,125 @@ static size_t isis_router_cap_fad_sub_tlv_len(const struct isis_router_cap_fad *
}
#endif /* ifndef FABRICD */
+static int isis_router_cap_srv6_msd_count(const struct isis_router_cap *router_cap)
+{
+ return !!router_cap->srv6_msd.max_seg_left_msd + !!router_cap->srv6_msd.max_end_pop_msd +
+ !!router_cap->srv6_msd.max_h_encaps_msd + !!router_cap->srv6_msd.max_end_d_msd;
+}
+
+/*
+ * Size of a sub-TLV of subtlv_sz bytes added to the Router Capability TLV of
+ * tlv_len bytes, including a new Router Capability TLV with a header of
+ * hdr_len bytes when the sub-TLV does not fit in the current one.
+ */
+static size_t isis_router_cap_subtlv_size(size_t *tlv_len, size_t hdr_len, size_t subtlv_sz)
+{
+ if (*tlv_len + subtlv_sz <= UINT8_MAX) {
+ *tlv_len += subtlv_sz;
+ return subtlv_sz;
+ }
+
+ *tlv_len = hdr_len + subtlv_sz;
+ return 2 + hdr_len + subtlv_sz;
+}
+
static size_t isis_router_cap_tlv_size(const struct isis_router_cap *router_cap)
{
- size_t sz = 2 + ISIS_ROUTER_CAP_SIZE;
-#ifndef FABRICD
- size_t fad_sz;
-#endif /* ifndef FABRICD */
+ size_t hdr_len = ISIS_ROUTER_CAP_SIZE;
+ size_t tlv_len = hdr_len;
+ size_t sz;
int nb_algo, nb_msd;
+ bool sr_algo_subtlv_present = false;
if ((router_cap->srgb.range_size != 0) && (router_cap->srgb.lower_bound != 0)) {
- sz += 2 + ISIS_SUBTLV_SID_LABEL_RANGE_SIZE;
- sz += 2 + ISIS_SUBTLV_SID_LABEL_SIZE;
+ tlv_len += 2 + ISIS_SUBTLV_SID_LABEL_RANGE_SIZE;
+ tlv_len += 2 + ISIS_SUBTLV_SID_LABEL_SIZE;
nb_algo = isis_tlvs_sr_algo_count(router_cap);
- if (nb_algo != 0)
- sz += 2 + nb_algo;
+ if (nb_algo != 0) {
+ tlv_len += 2 + nb_algo;
+ sr_algo_subtlv_present = true;
+ }
if ((router_cap->srlb.range_size != 0) && (router_cap->srlb.lower_bound != 0)) {
- sz += 2 + ISIS_SUBTLV_SID_LABEL_RANGE_SIZE;
- sz += 2 + ISIS_SUBTLV_SID_LABEL_SIZE;
+ tlv_len += 2 + ISIS_SUBTLV_SID_LABEL_RANGE_SIZE;
+ tlv_len += 2 + ISIS_SUBTLV_SID_LABEL_SIZE;
}
if (router_cap->msd != 0)
- sz += 2 + ISIS_SUBTLV_NODE_MSD_SIZE;
+ tlv_len += 2 + ISIS_SUBTLV_NODE_MSD_SIZE;
}
+ sz = 2 + tlv_len;
#ifndef FABRICD
for (int i = 0; i < SR_ALGORITHM_COUNT; i++) {
if (!router_cap->fads[i])
continue;
- fad_sz = 2 + isis_router_cap_fad_sub_tlv_len(router_cap->fads[i]);
- if (((sz + fad_sz) % 256) < (sz % 256))
- sz += 2 + ISIS_ROUTER_CAP_SIZE + fad_sz;
- else
- sz += fad_sz;
+ sz += isis_router_cap_subtlv_size(&tlv_len, hdr_len,
+ 2 + isis_router_cap_fad_sub_tlv_len(
+ router_cap->fads[i]));
}
#endif /* ifndef FABRICD */
if (router_cap->srv6_cap.is_srv6_capable) {
- sz += ISIS_SUBTLV_TYPE_FIELD_SIZE + ISIS_SUBTLV_LENGTH_FIELD_SIZE +
- ISIS_SUBTLV_SRV6_CAPABILITIES_SIZE;
+ sz += isis_router_cap_subtlv_size(&tlv_len, hdr_len,
+ ISIS_SUBTLV_TYPE_FIELD_SIZE +
+ ISIS_SUBTLV_LENGTH_FIELD_SIZE +
+ ISIS_SUBTLV_SRV6_CAPABILITIES_SIZE);
nb_algo = isis_tlvs_sr_algo_count(router_cap);
- if (nb_algo != 0)
- sz += ISIS_SUBTLV_TYPE_FIELD_SIZE + ISIS_SUBTLV_LENGTH_FIELD_SIZE + nb_algo;
+ if (nb_algo != 0 && !sr_algo_subtlv_present)
+ sz += isis_router_cap_subtlv_size(&tlv_len, hdr_len,
+ ISIS_SUBTLV_TYPE_FIELD_SIZE +
+ ISIS_SUBTLV_LENGTH_FIELD_SIZE + nb_algo);
- nb_msd = router_cap->srv6_msd.max_seg_left_msd +
- router_cap->srv6_msd.max_end_pop_msd +
- router_cap->srv6_msd.max_h_encaps_msd + router_cap->srv6_msd.max_end_d_msd;
+ nb_msd = isis_router_cap_srv6_msd_count(router_cap);
if (nb_msd != 0)
- sz += ISIS_SUBTLV_TYPE_FIELD_SIZE + ISIS_SUBTLV_LENGTH_FIELD_SIZE +
- (ISIS_SUBTLV_NODE_MSD_TYPE_SIZE + ISIS_SUBTLV_NODE_MSD_VALUE_SIZE) *
- nb_msd;
+ sz += isis_router_cap_subtlv_size(
+ &tlv_len, hdr_len,
+ ISIS_SUBTLV_TYPE_FIELD_SIZE + ISIS_SUBTLV_LENGTH_FIELD_SIZE +
+ (ISIS_SUBTLV_NODE_MSD_TYPE_SIZE + ISIS_SUBTLV_NODE_MSD_VALUE_SIZE) *
+ nb_msd);
}
return sz;
}
+static void pack_tlv_router_cap_header(const struct isis_router_cap *router_cap, struct stream *s)
+{
+ stream_put_ipv4(s, router_cap->router_id.s_addr);
+ stream_putc(s, router_cap->flags);
+}
+
+/*
+ * Return the position of the length of the Router Capability TLV to which a
+ * sub-TLV of subtlv_sz bytes is added: the current one at len_pos, or a new
+ * one when the sub-TLV does not fit in the current one.
+ */
+static size_t pack_tlv_router_cap_room(const struct isis_router_cap *router_cap,
+ struct stream *s, size_t len_pos, size_t subtlv_sz)
+{
+ size_t tlv_len = stream_get_endp(s) - len_pos - 1;
+
+ if (tlv_len + subtlv_sz <= UINT8_MAX)
+ return len_pos;
+
+ stream_putc_at(s, len_pos, tlv_len);
+
+ /* Add Router Capability TLV 242 with Router ID and Flags */
+ stream_putc(s, ISIS_TLV_ROUTER_CAPABILITY);
+ len_pos = stream_get_endp(s);
+ stream_putc(s, 0); /* Real length will be adjusted later */
+ pack_tlv_router_cap_header(router_cap, s);
+
+ return len_pos;
+}
+
static int pack_tlv_router_cap(const struct isis_router_cap *router_cap, struct stream *s)
{
size_t tlv_len, len_pos;
uint8_t nb_algo;
+ int nb_msd;
size_t subtlv_len, subtlv_len_pos;
bool sr_algo_subtlv_present = false;
@@ -4983,8 +5041,7 @@ static int pack_tlv_router_cap(const struct isis_router_cap *router_cap, struct
stream_putc(s, ISIS_TLV_ROUTER_CAPABILITY);
len_pos = stream_get_endp(s);
stream_putc(s, 0); /* Real length will be adjusted later */
- stream_put_ipv4(s, router_cap->router_id.s_addr);
- stream_putc(s, router_cap->flags);
+ pack_tlv_router_cap_header(router_cap, s);
/* Add SRGB if set as per RFC8667 section #3.1 */
if ((router_cap->srgb.range_size != 0) && (router_cap->srgb.lower_bound != 0)) {
@@ -5041,23 +5098,7 @@ static int pack_tlv_router_cap(const struct isis_router_cap *router_cap, struct
continue;
subtlv_len = isis_router_cap_fad_sub_tlv_len(fad);
-
- if ((stream_get_endp(s) - len_pos - 1) > 250) {
- /* Adjust TLV length which depends on subTLVs presence
- */
- tlv_len = stream_get_endp(s) - len_pos - 1;
- stream_putc_at(s, len_pos, tlv_len);
-
- /* Add Router Capability TLV 242 with Router ID and
- * Flags
- */
- stream_putc(s, ISIS_TLV_ROUTER_CAPABILITY);
- /* Real length will be adjusted later */
- len_pos = stream_get_endp(s);
- stream_putc(s, 0);
- stream_put_ipv4(s, router_cap->router_id.s_addr);
- stream_putc(s, router_cap->flags);
- }
+ len_pos = pack_tlv_router_cap_room(router_cap, s, len_pos, 2 + subtlv_len);
stream_putc(s, ISIS_SUBTLV_FAD);
stream_putc(s, subtlv_len); /* length will be filled later */
@@ -5104,6 +5145,10 @@ static int pack_tlv_router_cap(const struct isis_router_cap *router_cap, struct
/* Add SRv6 capabilities if set as per RFC 9352 section #2 */
if (router_cap->srv6_cap.is_srv6_capable) {
+ len_pos = pack_tlv_router_cap_room(router_cap, s, len_pos,
+ ISIS_SUBTLV_TYPE_FIELD_SIZE +
+ ISIS_SUBTLV_LENGTH_FIELD_SIZE +
+ ISIS_SUBTLV_SRV6_CAPABILITIES_SIZE);
stream_putc(s, ISIS_SUBTLV_SRV6_CAPABILITIES);
stream_putc(s, ISIS_SUBTLV_SRV6_CAPABILITIES_SIZE);
stream_putw(s, router_cap->srv6_cap.flags);
@@ -5116,6 +5161,10 @@ static int pack_tlv_router_cap(const struct isis_router_cap *router_cap, struct
if (!sr_algo_subtlv_present) {
nb_algo = isis_tlvs_sr_algo_count(router_cap);
if (nb_algo > 0) {
+ len_pos = pack_tlv_router_cap_room(router_cap, s, len_pos,
+ ISIS_SUBTLV_TYPE_FIELD_SIZE +
+ ISIS_SUBTLV_LENGTH_FIELD_SIZE +
+ nb_algo);
stream_putc(s, ISIS_SUBTLV_ALGORITHM);
stream_putc(s, nb_algo);
for (int i = 0; i < SR_ALGORITHM_COUNT; i++)
@@ -5125,10 +5174,13 @@ static int pack_tlv_router_cap(const struct isis_router_cap *router_cap, struct
}
/* And finish with MSDs if set as per RFC 9352 section #4 */
- if (router_cap->srv6_msd.max_seg_left_msd + router_cap->srv6_msd.max_end_pop_msd +
- router_cap->srv6_msd.max_h_encaps_msd +
- router_cap->srv6_msd.max_end_d_msd !=
- 0) {
+ nb_msd = isis_router_cap_srv6_msd_count(router_cap);
+ if (nb_msd != 0) {
+ len_pos = pack_tlv_router_cap_room(
+ router_cap, s, len_pos,
+ ISIS_SUBTLV_TYPE_FIELD_SIZE + ISIS_SUBTLV_LENGTH_FIELD_SIZE +
+ (ISIS_SUBTLV_NODE_MSD_TYPE_SIZE + ISIS_SUBTLV_NODE_MSD_VALUE_SIZE) *
+ nb_msd);
stream_putc(s, ISIS_SUBTLV_NODE_MSD);
subtlv_len_pos = stream_get_endp(s);