diff --git a/lib/mlag.c b/lib/mlag.c index 1daf290725..733dd41ea8 100644 --- a/lib/mlag.c +++ b/lib/mlag.c @@ -81,22 +81,33 @@ char *mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf, size_t size) } -int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg) +int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg, + size_t *length) { - if (s == NULL || msg == NULL) +#define LIB_MLAG_HDR_LENGTH 8 + *length = stream_get_endp(s); + + if (s == NULL || msg == NULL || *length < LIB_MLAG_HDR_LENGTH) return -1; + *length -= LIB_MLAG_HDR_LENGTH; + STREAM_GETL(s, msg->msg_type); STREAM_GETW(s, msg->data_len); STREAM_GETW(s, msg->msg_cnt); + return 0; stream_failure: return -1; } -int mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg) +#define MLAG_MROUTE_ADD_LENGTH \ + (VRF_NAMSIZ + INTERFACE_NAMSIZ + 4 + 4 + 4 + 4 + 1 + 1 + 4) + +int mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg, + size_t *length) { - if (s == NULL || msg == NULL) + if (s == NULL || msg == NULL || *length < MLAG_MROUTE_ADD_LENGTH) return -1; STREAM_GET(msg->vrf_name, s, VRF_NAMSIZ); @@ -108,14 +119,18 @@ int mlag_lib_decode_mroute_add(struct stream *s, struct mlag_mroute_add *msg) STREAM_GETC(s, msg->am_i_dual_active); STREAM_GETL(s, msg->vrf_id); STREAM_GET(msg->intf_name, s, INTERFACE_NAMSIZ); + return 0; stream_failure: return -1; } -int mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg) +#define MLAG_MROUTE_DEL_LENGTH (VRF_NAMSIZ + INTERFACE_NAMSIZ + 4 + 4 + 4 + 4) + +int mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg, + size_t *length) { - if (s == NULL || msg == NULL) + if (s == NULL || msg == NULL || *length < MLAG_MROUTE_DEL_LENGTH) return -1; STREAM_GET(msg->vrf_name, s, VRF_NAMSIZ); @@ -124,6 +139,7 @@ int mlag_lib_decode_mroute_del(struct stream *s, struct mlag_mroute_del *msg) STREAM_GETL(s, msg->owner_id); STREAM_GETL(s, msg->vrf_id); STREAM_GET(msg->intf_name, s, INTERFACE_NAMSIZ); + return 0; stream_failure: return -1; diff --git a/lib/mlag.h b/lib/mlag.h index c531fb5b68..37bb3aa6db 100644 --- a/lib/mlag.h +++ b/lib/mlag.h @@ -125,11 +125,14 @@ struct mlag_msg { extern char *mlag_role2str(enum mlag_role role, char *buf, size_t size); extern char *mlag_lib_msgid_to_str(enum mlag_msg_type msg_type, char *buf, size_t size); -extern int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg); +extern int mlag_lib_decode_mlag_hdr(struct stream *s, struct mlag_msg *msg, + size_t *length); extern int mlag_lib_decode_mroute_add(struct stream *s, - struct mlag_mroute_add *msg); + struct mlag_mroute_add *msg, + size_t *length); extern int mlag_lib_decode_mroute_del(struct stream *s, - struct mlag_mroute_del *msg); + struct mlag_mroute_del *msg, + size_t *length); extern int mlag_lib_decode_mlag_status(struct stream *s, struct mlag_status *msg); extern int mlag_lib_decode_vxlan_update(struct stream *s, diff --git a/pimd/pim_mlag.c b/pimd/pim_mlag.c index 321745590d..0a6ff41114 100644 --- a/pimd/pim_mlag.c +++ b/pimd/pim_mlag.c @@ -765,8 +765,9 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len) struct mlag_msg mlag_msg; char buf[ZLOG_FILTER_LENGTH_MAX]; int rc = 0; + size_t length; - rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg); + rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg, &length); if (rc) return (rc); @@ -805,7 +806,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len) case MLAG_MROUTE_ADD: { struct mlag_mroute_add msg; - rc = mlag_lib_decode_mroute_add(s, &msg); + rc = mlag_lib_decode_mroute_add(s, &msg, &length); if (rc) return (rc); pim_mlag_process_mroute_add(msg); @@ -813,7 +814,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len) case MLAG_MROUTE_DEL: { struct mlag_mroute_del msg; - rc = mlag_lib_decode_mroute_del(s, &msg); + rc = mlag_lib_decode_mroute_del(s, &msg, &length); if (rc) return (rc); pim_mlag_process_mroute_del(msg); @@ -823,8 +824,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len) int i; for (i = 0; i < mlag_msg.msg_cnt; i++) { - - rc = mlag_lib_decode_mroute_add(s, &msg); + rc = mlag_lib_decode_mroute_add(s, &msg, &length); if (rc) return (rc); pim_mlag_process_mroute_add(msg); @@ -835,8 +835,7 @@ int pim_zebra_mlag_handle_msg(struct stream *s, int len) int i; for (i = 0; i < mlag_msg.msg_cnt; i++) { - - rc = mlag_lib_decode_mroute_del(s, &msg); + rc = mlag_lib_decode_mroute_del(s, &msg, &length); if (rc) return (rc); pim_mlag_process_mroute_del(msg); diff --git a/zebra/zebra_mlag.c b/zebra/zebra_mlag.c index 5b721a8eac..edd71b9f77 100644 --- a/zebra/zebra_mlag.c +++ b/zebra/zebra_mlag.c @@ -667,14 +667,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) int n_len = 0; int rc = 0; char buf[ZLOG_FILTER_LENGTH_MAX]; + size_t length; if (IS_ZEBRA_DEBUG_MLAG) zlog_debug("%s: Entering..", __func__); - rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg); + rc = mlag_lib_decode_mlag_hdr(s, &mlag_msg, &length); if (rc) return rc; + memset(tmp_buf, 0, ZEBRA_MLAG_BUF_LIMIT); + if (IS_ZEBRA_DEBUG_MLAG) zlog_debug("%s: Mlag ProtoBuf encoding of message:%s, len:%d", __func__, @@ -688,9 +691,10 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) ZebraMlagMrouteAdd pay_load = ZEBRA_MLAG_MROUTE_ADD__INIT; uint32_t vrf_name_len = 0; - rc = mlag_lib_decode_mroute_add(s, &msg); + rc = mlag_lib_decode_mroute_add(s, &msg, &length); if (rc) return rc; + vrf_name_len = strlen(msg.vrf_name) + 1; pay_load.vrf_name = XMALLOC(MTYPE_MLAG_PBUF, vrf_name_len); strlcpy(pay_load.vrf_name, msg.vrf_name, vrf_name_len); @@ -720,7 +724,7 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) ZebraMlagMrouteDel pay_load = ZEBRA_MLAG_MROUTE_DEL__INIT; uint32_t vrf_name_len = 0; - rc = mlag_lib_decode_mroute_del(s, &msg); + rc = mlag_lib_decode_mroute_del(s, &msg, &length); if (rc) return rc; vrf_name_len = strlen(msg.vrf_name) + 1; @@ -749,18 +753,18 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) ZebraMlagMrouteAddBulk Bulk_msg = ZEBRA_MLAG_MROUTE_ADD_BULK__INIT; ZebraMlagMrouteAdd **pay_load = NULL; - int i; bool cleanup = false; + uint32_t i, actual; Bulk_msg.n_mroute_add = mlag_msg.msg_cnt; pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteAdd *) * mlag_msg.msg_cnt); - for (i = 0; i < mlag_msg.msg_cnt; i++) { + for (i = 0, actual = 0; i < mlag_msg.msg_cnt; i++, actual++) { uint32_t vrf_name_len = 0; - rc = mlag_lib_decode_mroute_add(s, &msg); + rc = mlag_lib_decode_mroute_add(s, &msg, &length); if (rc) { cleanup = true; break; @@ -796,8 +800,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) tmp_buf); } - for (i = 0; i < mlag_msg.msg_cnt; i++) { - XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name); + for (i = 0; i < actual; i++) { + /* + * The mlag_lib_decode_mroute_add can + * fail to properly decode and cause nothing + * to be allocated. Prevent a crash + */ + if (!pay_load[i]) + continue; + + if (pay_load[i]->vrf_name) + XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name); if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE && pay_load[i]->intf_name) XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name); @@ -812,18 +825,18 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) ZebraMlagMrouteDelBulk Bulk_msg = ZEBRA_MLAG_MROUTE_DEL_BULK__INIT; ZebraMlagMrouteDel **pay_load = NULL; - int i; bool cleanup = false; + uint32_t i, actual; Bulk_msg.n_mroute_del = mlag_msg.msg_cnt; pay_load = XMALLOC(MTYPE_MLAG_PBUF, sizeof(ZebraMlagMrouteDel *) * mlag_msg.msg_cnt); - for (i = 0; i < mlag_msg.msg_cnt; i++) { + for (i = 0, actual = 0; i < mlag_msg.msg_cnt; i++, actual++) { uint32_t vrf_name_len = 0; - rc = mlag_lib_decode_mroute_del(s, &msg); + rc = mlag_lib_decode_mroute_del(s, &msg, &length); if (rc) { cleanup = true; break; @@ -858,8 +871,17 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) tmp_buf); } - for (i = 0; i < mlag_msg.msg_cnt; i++) { - XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name); + for (i = 0; i < actual; i++) { + /* + * The mlag_lib_decode_mroute_add can + * fail to properly decode and cause nothing + * to be allocated. Prevent a crash + */ + if (!pay_load[i]) + continue; + + if (pay_load[i]->vrf_name) + XFREE(MTYPE_MLAG_PBUF, pay_load[i]->vrf_name); if (pay_load[i]->owner_id == MLAG_OWNER_INTERFACE && pay_load[i]->intf_name) XFREE(MTYPE_MLAG_PBUF, pay_load[i]->intf_name); @@ -915,6 +937,15 @@ int zebra_mlag_protobuf_encode_client_data(struct stream *s, uint32_t *msg_type) return len; } +static void zebra_fill_protobuf_msg(struct stream *s, char *name, int len) +{ + int str_len = strlen(name) + 1; + + stream_put(s, name, str_len); + /* Fill the rest with Null Character for aligning */ + stream_put(s, NULL, len - str_len); +} + int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, uint32_t len) { @@ -966,7 +997,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, /* No Batching */ stream_putw(s, MLAG_MSG_NO_BATCH); /* Actual Data */ - stream_put(s, msg->peerlink, INTERFACE_NAMSIZ); + zebra_fill_protobuf_msg(s, msg->peerlink, + INTERFACE_NAMSIZ); stream_putl(s, msg->my_role); stream_putl(s, msg->peer_state); zebra_mlag_status_update__free_unpacked(msg, NULL); @@ -1003,7 +1035,7 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, /* No Batching */ stream_putw(s, MLAG_MSG_NO_BATCH); /* Actual Data */ - stream_put(s, msg->vrf_name, VRF_NAMSIZ); + zebra_fill_protobuf_msg(s, msg->vrf_name, VRF_NAMSIZ); stream_putl(s, msg->source_ip); stream_putl(s, msg->group_ip); @@ -1013,7 +1045,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, stream_putc(s, msg->am_i_dual_active); stream_putl(s, msg->vrf_id); if (msg->owner_id == MLAG_OWNER_INTERFACE) - stream_put(s, msg->intf_name, INTERFACE_NAMSIZ); + zebra_fill_protobuf_msg(s, msg->intf_name, + INTERFACE_NAMSIZ); else stream_put(s, NULL, INTERFACE_NAMSIZ); zebra_mlag_mroute_add__free_unpacked(msg, NULL); @@ -1032,15 +1065,15 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, /* No Batching */ stream_putw(s, MLAG_MSG_NO_BATCH); /* Actual Data */ - stream_put(s, msg->vrf_name, VRF_NAMSIZ); + zebra_fill_protobuf_msg(s, msg->vrf_name, VRF_NAMSIZ); stream_putl(s, msg->source_ip); stream_putl(s, msg->group_ip); - stream_putl(s, msg->group_ip); stream_putl(s, msg->owner_id); stream_putl(s, msg->vrf_id); if (msg->owner_id == MLAG_OWNER_INTERFACE) - stream_put(s, msg->intf_name, INTERFACE_NAMSIZ); + zebra_fill_protobuf_msg(s, msg->intf_name, + INTERFACE_NAMSIZ); else stream_put(s, NULL, INTERFACE_NAMSIZ); zebra_mlag_mroute_del__free_unpacked(msg, NULL); @@ -1067,7 +1100,8 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, msg = Bulk_msg->mroute_add[i]; - stream_put(s, msg->vrf_name, VRF_NAMSIZ); + zebra_fill_protobuf_msg(s, msg->vrf_name, + VRF_NAMSIZ); stream_putl(s, msg->source_ip); stream_putl(s, msg->group_ip); stream_putl(s, msg->cost_to_rp); @@ -1076,8 +1110,9 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, stream_putc(s, msg->am_i_dual_active); stream_putl(s, msg->vrf_id); if (msg->owner_id == MLAG_OWNER_INTERFACE) - stream_put(s, msg->intf_name, - INTERFACE_NAMSIZ); + zebra_fill_protobuf_msg( + s, msg->intf_name, + INTERFACE_NAMSIZ); else stream_put(s, NULL, INTERFACE_NAMSIZ); } @@ -1106,14 +1141,16 @@ int zebra_mlag_protobuf_decode_message(struct stream *s, uint8_t *data, msg = Bulk_msg->mroute_del[i]; - stream_put(s, msg->vrf_name, VRF_NAMSIZ); + zebra_fill_protobuf_msg(s, msg->vrf_name, + VRF_NAMSIZ); stream_putl(s, msg->source_ip); stream_putl(s, msg->group_ip); stream_putl(s, msg->owner_id); stream_putl(s, msg->vrf_id); if (msg->owner_id == MLAG_OWNER_INTERFACE) - stream_put(s, msg->intf_name, - INTERFACE_NAMSIZ); + zebra_fill_protobuf_msg( + s, msg->intf_name, + INTERFACE_NAMSIZ); else stream_put(s, NULL, INTERFACE_NAMSIZ); }