Skip to content

Commit

Permalink
pbrd: Fix nearly impossible truncation
Browse files Browse the repository at this point in the history
Since we are writing into the name field which is PBR_MAP_NAMELEN
size, we are expecting this to field to be at max 100 bytes.
Newer compilers understand that the %s portion may be up to
100 bytes( because of the size of the string.  The %u portion
is expected to be 10 bytes.  So in `theory` there are situations
where we might truncate.  The reality this is never going to
happen( who is going to create a nexthop group name that is
over say 30 characters? ).  As such we are expecting the
calling function to subtract 10 from the size_t l before
we pass it in to get around this new gcc fun.

Fixes: #2163
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
  • Loading branch information
donaldsharp committed May 3, 2018
1 parent d71b056 commit 29d5a14
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
14 changes: 13 additions & 1 deletion pbrd/pbr_nht.c
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,18 @@ void pbr_nht_change_group(const char *name)
pbr_nht_install_nexthop_group(pnhgc, nhgc->nhg);
}

/*
* Since we are writing into the name field which is PBR_MAP_NAMELEN
* size, we are expecting this to field to be at max 100 bytes.
* Newer compilers understand that the %s portion may be up to
* 100 bytes( because of the size of the string. The %u portion
* is expected to be 10 bytes. So in `theory` there are situations
* where we might truncate. The reality this is never going to
* happen( who is going to create a nexthop group name that is
* over say 30 characters? ). As such we are expecting the
* calling function to subtract 10 from the size_t l before
* we pass it in to get around this new gcc fun.
*/
char *pbr_nht_nexthop_make_name(char *name, size_t l,
uint32_t seqno, char *buffer)
{
Expand All @@ -485,7 +497,7 @@ void pbr_nht_add_individual_nexthop(struct pbr_map_sequence *pbrms)
struct pbr_nexthop_cache lookup;

memset(&find, 0, sizeof(find));
pbr_nht_nexthop_make_name(pbrms->parent->name, PBR_MAP_NAMELEN,
pbr_nht_nexthop_make_name(pbrms->parent->name, PBR_MAP_NAMELEN - 10,
pbrms->seqno, find.name);
if (!pbrms->internal_nhg_name)
pbrms->internal_nhg_name = XSTRDUP(MTYPE_TMP, find.name);
Expand Down
4 changes: 2 additions & 2 deletions pbrd/pbr_vty.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
if (pbrms->nhg)
nh = nexthop_exists(pbrms->nhg, &nhop);
else {
char buf[100];
char buf[PBR_MAP_NAMELEN];

if (no) {
vty_out(vty, "No nexthops to delete");
Expand All @@ -280,7 +280,7 @@ DEFPY(pbr_map_nexthop, pbr_map_nexthop_cmd,
pbrms->internal_nhg_name =
XSTRDUP(MTYPE_TMP,
pbr_nht_nexthop_make_name(pbrms->parent->name,
PBR_MAP_NAMELEN,
PBR_MAP_NAMELEN - 10,
pbrms->seqno,
buf));
nh = NULL;
Expand Down

0 comments on commit 29d5a14

Please sign in to comment.