Skip to content

Commit

Permalink
zebra: prevent from creating pic nhe resolving over themselves
Browse files Browse the repository at this point in the history 
There is no control on the returned nexthop group entry, when
finding pic contexts. Actually the pic context can resolve
over itself, and this may lead to stack overflow:

The below can be found by generalizing the search of pic nhe
for all nexthops and not only for srv6 contexts.

> root@ubuntu2204hwe:~/frr# AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==247856==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe4e6dcff8 (pc 0x561e05bb5653 bp 0x7ffe4e6dd020 sp 0x7ffe4e6dd000 T0)
>     #0 0x561e05bb5653 in zebra_nhg_install_kernel zebra/zebra_nhg.c:3310
>     FRRouting#1 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#2 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#3 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#4 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#5 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#6 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#7 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#8 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#9 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#10 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#11 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#12 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#13 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329
>     FRRouting#14 0x561e05bb572d in zebra_nhg_install_kernel zebra/zebra_nhg.c:3329

Fix this by not returning a nexthop group entry when creation is
necessary for pic context.
Add a check when the pic creation is not needed and the returned
nhe has the same identifier as the requested nhe.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
@pguibert6WIND
pguibert6WIND committed Jan 7, 2025
1 parent d394228 commit 1b870c2
Showing 1 changed file with 40 additions and 9 deletions.
49 changes: 40 additions & 9 deletions zebra/zebra_nhg.c
View file
Original file line number Diff line number Diff line change
@@ -686,7 +686,7 @@ static struct nhg_hash_entry *handle_recursive_depend(struct nhg_hash_entry *nhe
depend ? depend->id : 0);

if (depend) {
if (depend->id == nhe->id) {
if (pic && depend->id == nhe->id) {
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
zlog_debug("%s: NHE %d resolved against itself",
__func__, nhe->id);
@@ -722,10 +722,10 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */
bool created = false;
bool createdPic = false;
bool recursive = false;
struct nhg_hash_entry *newnhe, *backup_nhe, *pic_nhe;
struct nhg_hash_entry *newnhe, *backup_nhe, *pic_nhe = NULL,
*temp_nhe = NULL;
struct nexthop *nh = NULL;


if (lookup->id)
(*nhe) = zebra_nhg_lookup_id(lookup->id);
else
@@ -805,7 +805,14 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */
if (nh->next == NULL && newnhe->id < ZEBRA_NHG_PROTO_LOWER) {
if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RECURSIVE)) {
/* Single recursive nexthop */
handle_recursive_depend(newnhe, nh->resolved, afi, pic);
temp_nhe = handle_recursive_depend(newnhe, nh->resolved,
afi, pic);
if (pic && temp_nhe == NULL) {
/* recursive dependency failed */
zebra_nhg_free(newnhe);
*nhe = NULL;
return 0;
}
recursive = true;
}
} else {
@@ -819,10 +826,16 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */
NEXTHOP_FLAG_RECURSIVE) ?
"(R)" : "");

depends_find_add(newnhe, nh, afi, from_dplane, pic);
temp_nhe = depends_find_add(newnhe, nh, afi,
from_dplane, pic);
if (pic && temp_nhe == NULL) {
/* recursive dependency failed */
zebra_nhg_free(newnhe);
*nhe = NULL;
return 0;
}
}
}

if (recursive)
SET_FLAG(newnhe->flags, NEXTHOP_GROUP_RECURSIVE);

@@ -853,7 +866,8 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */
__func__, nh);

/* Single recursive nexthop */
handle_recursive_depend(backup_nhe, nh->resolved, afi, pic);
temp_nhe = handle_recursive_depend(backup_nhe, nh->resolved,
afi, pic);
recursive = true;
} else {
/* One or more backup NHs */
@@ -865,7 +879,14 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */
NEXTHOP_FLAG_RECURSIVE) ?
"(R)" : "");

depends_find_add(backup_nhe, nh, afi, from_dplane, pic);
temp_nhe = depends_find_add(backup_nhe, nh, afi,
from_dplane, pic);
if (pic && temp_nhe == NULL) {
/* recursive dependency failed */
zebra_nhg_free(newnhe);
*nhe = NULL;
return 0;
}
}
}

@@ -954,6 +975,16 @@ bool zebra_pic_nhe_find(struct nhg_hash_entry **pic_nhe, /* return value */

created = zebra_nhe_find(&picnhe, &pic_nh_lookup, NULL, afi, from_dplane, true);

if (created == false && (picnhe == NULL || picnhe->id == nhe->id)) {
if (IS_ZEBRA_DEBUG_NHG_DETAIL) {
zlog_debug("%s: nhe %u failed to find a PIC NHE (recursive loop?)",
__func__, nhe->id);
*pic_nhe = NULL;
}
if (pic_nh_lookup.nhg.nexthop)
nexthops_free(pic_nh_lookup.nhg.nexthop);
return false;
}
*pic_nhe = picnhe;
if (pic_nh_lookup.nhg.nexthop)
nexthops_free(pic_nh_lookup.nhg.nexthop);
@@ -1614,7 +1645,7 @@ static struct nhg_hash_entry *depends_find_add(struct nhg_hash_entry *nhe,
__func__, nh, depend);

if (depend) {
if (depend->id == nhe->id) {
if (pic && depend->id == nhe->id) {
if (IS_ZEBRA_DEBUG_RIB_DETAILED)
zlog_debug("%s: NHE %d resolved against itself",
__func__, nhe->id);

0 comments on commit 1b870c2

Please sign in to comment.