]> git.proxmox.com Git - mirror_frr.git/commitdiff
zebra: fix multiple bfd buffer issues
authorQuentin Young <qlyoung@cumulusnetworks.com>
Sat, 4 Jan 2020 02:18:49 +0000 (21:18 -0500)
committerQuentin Young <qlyoung@cumulusnetworks.com>
Wed, 15 Jan 2020 17:50:12 +0000 (12:50 -0500)
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>
zebra/zebra_ptm.c

index 46f138552059f6f9edfc03287cc3bc991c2ec22f..cefbae54589e00721b246326763f3f7152948a68 100644 (file)
@@ -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);