From: Daniel Walton <[email protected]>
Signed-off-by: Daniel Walton <[email protected]>
---
bgpd/bgp_packet.c | 35 +++++++++++++++++++++++++++++++----
bgpd/bgpd.c | 14 +++++++++++++-
bgpd/bgpd.h | 1 +
3 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index b6744f9..842063f 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -151,6 +151,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;
@@ -169,9 +171,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. */
@@ -205,6 +210,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)
@@ -335,6 +356,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);
@@ -345,8 +368,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 bdf5e30..df153fa 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -886,7 +886,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 543b6ed..e318e37 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -315,6 +315,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
--
1.9.1
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
