]> git.proxmox.com Git - mirror_frr.git/commitdiff
pbrd: Fix nearly impossible truncation
authorDonald Sharp <sharpd@cumulusnetworks.com>
Thu, 3 May 2018 00:12:31 +0000 (20:12 -0400)
committerDonald Sharp <sharpd@cumulusnetworks.com>
Thu, 3 May 2018 00:14:36 +0000 (20:14 -0400)
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>
pbrd/pbr_nht.c
pbrd/pbr_vty.c

index 1ccf3ebffac8255377905d617f644703efbcf71f..5be96e86d00c030abd80036423b86586493e8941 100644 (file)
@@ -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)
 {
@@ -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);
index 03f2104835ff46463fb12dee2614a65d115a7536..ba5c49ad5c7c1ec5ce5bda577fdb089769751099 100644 (file)
@@ -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");
@@ -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;