From: Quentin Young <qlyoung@cumulusnetworks.com>
Date: Sat, 4 Jan 2020 02:18:49 +0000 (-0500)
Subject: zebra: fix multiple bfd buffer issues
X-Git-Tag: frr-7.2.1~4^2~9
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=a4f1cc55843b5596044ab691c2fd21104afa9e7f;p=matthieu%2Ffrr.git

zebra: fix multiple bfd buffer issues

Whatever this BFD re-transmission function is had a few problems.

1. Used memcpy instead of the (more concise) stream APIs, which include
   bounds checking.
2. Did not sufficiently check packet sizes.

Actually, 2) is mitigated but is still a problem, because the BFD header
is 2 bytes larger than the "normal" ZAPI header, while the overall
message size remains the same. So if the source message being duplicated
is actually right up against the ZAPI_MAX_PACKET_SIZ, you still can't
fit the whole message into your duplicated message. I have no idea what
the intent was here but at least there's a warning if it happens now.

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
---

diff --git a/zebra/zebra_ptm.c b/zebra/zebra_ptm.c
index 46f1385520..cefbae5458 100644
--- a/zebra/zebra_ptm.c
+++ b/zebra/zebra_ptm.c
@@ -1402,37 +1402,25 @@ static void _zebra_ptm_reroute(struct zserv *zs, struct zebra_vrf *zvrf,
 			       struct stream *msg, uint32_t command)
 {
 	struct stream *msgc;
-	size_t zmsglen, zhdrlen;
+	char buf[ZEBRA_MAX_PACKET_SIZ];
 	pid_t ppid;
 
-	/*
-	 * Don't modify message in the zebra API. In order to do that we
-	 * need to allocate a new message stream and copy the message
-	 * provided by zebra.
-	 */
+	/* Create BFD header */
 	msgc = stream_new(ZEBRA_MAX_PACKET_SIZ);
-	if (msgc == NULL) {
-		zlog_debug("%s: not enough memory", __func__);
-		return;
-	}
-
-	/* Calculate our header size plus the message contents. */
-	zhdrlen = ZEBRA_HEADER_SIZE + sizeof(uint32_t);
-	zmsglen = msg->endp - msg->getp;
-	memcpy(msgc->data + zhdrlen, msg->data + msg->getp, zmsglen);
-
-	/*
-	 * The message type will be BFD_DEST_REPLY so we can use only
-	 * one callback at the `bfdd` side, however the real command
-	 * number will be included right after the zebra header.
-	 */
 	zclient_create_header(msgc, ZEBRA_BFD_DEST_REPLAY, zvrf->vrf->vrf_id);
 	stream_putl(msgc, command);
 
-	/* Update the data pointers. */
-	msgc->getp = 0;
-	msgc->endp = zhdrlen + zmsglen;
-	stream_putw_at(msgc, 0, stream_get_endp(msgc));
+	if (STREAM_READABLE(msg) > STREAM_WRITEABLE(msgc)) {
+		zlog_warn("Cannot fit extended BFD header plus original message contents into ZAPI packet; dropping message");
+		goto stream_failure;
+	}
+
+	/* Copy original message, excluding header, into new message */
+	stream_get_from(buf, msg, stream_get_getp(msg), STREAM_READABLE(msg));
+	stream_put(msgc, buf, STREAM_READABLE(msg));
+
+	/* Update length field */
+	stream_putw_at(msgc, 0, STREAM_READABLE(msgc));
 
 	zebra_ptm_send_bfdd(msgc);