BGP crashes if attributes alone consume > 4096 bytes

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index ec7b0d60b7efeffc8d58b0c8bce53dfe8c655aa7..524c756113c95fc970d9fac97cfb5ed752c57255 100644 (file)
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -153,6 +153,8 @@ bgp_update_packet (struct peer *peer, afi_t afi, safi_t safi)
   struct bgp_info *binfo = NULL;
   bgp_size_t total_attr_len = 0;
   unsigned long attrlen_pos = 0;
+  int space_remaining = 0;
+  int space_needed = 0;
   size_t mpattrlen_pos = 0;
   size_t mpattr_pos = 0;
 
@@ -171,9 +173,12 @@ bgp_update_packet (struct peer *peer, afi_t afi, safi_t safi)
       if (adv->binfo)
         binfo = adv->binfo;
 
+      space_remaining = STREAM_CONCAT_REMAIN (s, snlri, STREAM_SIZE(s)) -
+                        BGP_MAX_PACKET_SIZE_OVERFLOW;
+      space_needed = BGP_NLRI_LENGTH + PSIZE (rn->p.prefixlen);
+
       /* When remaining space can't include NLRI and it's length.  */
-      if (STREAM_CONCAT_REMAIN (s, snlri, STREAM_SIZE(s)) <=
-         (BGP_NLRI_LENGTH + PSIZE (rn->p.prefixlen)))
+      if (space_remaining < space_needed)
        break;
 
       /* If packet is empty, set attribute. */
@@ -207,6 +212,22 @@ bgp_update_packet (struct peer *peer, afi_t afi, safi_t safi)
                                                 adv->baa->attr,
                                                 NULL, afi, safi,
                                                 from, NULL, NULL);
+
+          space_remaining = STREAM_CONCAT_REMAIN (s, snlri, STREAM_SIZE(s)) -
+                            BGP_MAX_PACKET_SIZE_OVERFLOW;
+          space_needed = BGP_NLRI_LENGTH + PSIZE (rn->p.prefixlen);
+
+          /* If the attributes alone do not leave any room for NLRI then
+           * return */
+          if (space_remaining < space_needed)
+            {
+              zlog_err ("%s cannot send UPDATE, the attributes do not leave "
+                        "room for NLRI", peer->host);
+              /* Flush the FIFO update queue */
+              while (adv)
+                adv = bgp_advertise_clean (peer, adv->adj, afi, safi);
+              return NULL;
+            }
        }
 
       if (afi == AFI_IP && safi == SAFI_UNICAST)
@@ -341,6 +362,8 @@ bgp_withdraw_packet (struct peer *peer, afi_t afi, safi_t safi)
   size_t attrlen_pos = 0;
   size_t mplen_pos = 0;
   u_char first_time = 1;
+  int space_remaining = 0;
+  int space_needed = 0;
 
   s = peer->work;
   stream_reset (s);
@@ -351,8 +374,12 @@ bgp_withdraw_packet (struct peer *peer, afi_t afi, safi_t safi)
       adj = adv->adj;
       rn = adv->rn;
 
-      if (STREAM_REMAIN (s)
-         < (BGP_NLRI_LENGTH + BGP_TOTAL_ATTR_LEN + PSIZE (rn->p.prefixlen)))
+      space_remaining = STREAM_REMAIN (s) -
+                        BGP_MAX_PACKET_SIZE_OVERFLOW;
+      space_needed = (BGP_NLRI_LENGTH + BGP_TOTAL_ATTR_LEN +
+                      PSIZE (rn->p.prefixlen));
+
+      if (space_remaining < space_needed)
        break;
 
       if (stream_empty (s))

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 653630e29aab620e07e5cf6f39475e85eb7b4b52..b205df262dd043c3f92f8968dd5edbd86f92fbd1 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -873,7 +873,19 @@ peer_new (struct bgp *bgp)
   /* Create buffers.  */
   peer->ibuf = stream_new (BGP_MAX_PACKET_SIZE);
   peer->obuf = stream_fifo_new ();
-  peer->work = stream_new (BGP_MAX_PACKET_SIZE);
+
+  /* We use a larger buffer for peer->work in the event that:
+   * - We RX a BGP_UPDATE where the attributes alone are just
+   *   under BGP_MAX_PACKET_SIZE
+   * - The user configures an outbound route-map that does many as-path
+   *   prepends or adds many communities.  At most they can have CMD_ARGC_MAX
+   *   args in a route-map so there is a finite limit on how large they can
+   *   make the attributes.
+   *
+   * Having a buffer with BGP_MAX_PACKET_SIZE_OVERFLOW allows us to avoid bounds
+   * checking for every single attribute as we construct an UPDATE.
+   */
+  peer->work = stream_new (BGP_MAX_PACKET_SIZE + BGP_MAX_PACKET_SIZE_OVERFLOW);
   peer->scratch = stream_new (BGP_MAX_PACKET_SIZE);
 
 

diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index 3d4e03e5f1453c28e3815d42aaa0f431b0f906c8..2e0419d3034b38e8fb6a44298e5a5aa005167dbd 100644 (file)
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -329,6 +329,7 @@ typedef enum
 #define BGP_MARKER_SIZE                                16
 #define BGP_HEADER_SIZE                                19
 #define BGP_MAX_PACKET_SIZE                   4096
+#define BGP_MAX_PACKET_SIZE_OVERFLOW          1024
 
 /* BGP neighbor structure. */
 struct peer