1From db24300d56ad5831d9f6e4545ff2999b99e71bac Mon Sep 17 00:00:00 2001 2From: Mark Stapp <mstapp@nvidia.com> 3Date: Thu, 8 Sep 2022 16:14:36 -0400 4Subject: [PATCH] bgpd: avoid notify race between io and main pthreads 5 6The "bgp_notify_" apis in bgp_packet.c generate a notification 7to a peer, usually during error handling. The io pthread wants 8to send notifications in a couple of cases during early 9received-packet validation - but the existing api interacts 10with the peer struct itself, and that's not safe. 11 12Add a new api for use by the io pthread, and adjust the main 13notify api so that it can avoid touching the peer struct. 14 15Signed-off-by: Mark Stapp <mstapp@nvidia.com> 16 17CVE: CVE-2022-37035 18 19Upstream-Status: Backport 20[https://github.com/FRRouting/frr/commit/71ca5b09bc71e8cbe38177cf41e83fe164e52eee] 21 22Signed-off-by: Yi Zhao <yi.zhao@windriver.com> 23--- 24 bgpd/bgp_io.c | 17 ++++++++--------- 25 bgpd/bgp_packet.c | 32 ++++++++++++++++++++++++++++---- 26 bgpd/bgp_packet.h | 2 ++ 27 3 files changed, 38 insertions(+), 13 deletions(-) 28 29diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c 30index 9b5a31f28..c736d02db 100644 31--- a/bgpd/bgp_io.c 32+++ b/bgpd/bgp_io.c 33@@ -37,7 +37,7 @@ 34 #include "bgpd/bgp_debug.h" // for bgp_debug_neighbor_events, bgp_type_str 35 #include "bgpd/bgp_errors.h" // for expanded error reference information 36 #include "bgpd/bgp_fsm.h" // for BGP_EVENT_ADD, bgp_event 37-#include "bgpd/bgp_packet.h" // for bgp_notify_send_with_data, bgp_notify... 38+#include "bgpd/bgp_packet.h" // for bgp_notify_io_invalid... 39 #include "bgpd/bgp_trace.h" // for frrtraces 40 #include "bgpd/bgpd.h" // for peer, BGP_MARKER_SIZE, bgp_master, bm 41 /* clang-format on */ 42@@ -526,8 +526,8 @@ static bool validate_header(struct peer *peer) 43 return false; 44 45 if (memcmp(m_correct, m_rx, BGP_MARKER_SIZE) != 0) { 46- bgp_notify_send(peer, BGP_NOTIFY_HEADER_ERR, 47- BGP_NOTIFY_HEADER_NOT_SYNC); 48+ bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR, 49+ BGP_NOTIFY_HEADER_NOT_SYNC, NULL, 0); 50 return false; 51 } 52 53@@ -547,9 +547,8 @@ static bool validate_header(struct peer *peer) 54 zlog_debug("%s unknown message type 0x%02x", peer->host, 55 type); 56 57- bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR, 58- BGP_NOTIFY_HEADER_BAD_MESTYPE, &type, 59- 1); 60+ bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR, 61+ BGP_NOTIFY_HEADER_BAD_MESTYPE, &type, 1); 62 return false; 63 } 64 65@@ -574,9 +573,9 @@ static bool validate_header(struct peer *peer) 66 67 uint16_t nsize = htons(size); 68 69- bgp_notify_send_with_data(peer, BGP_NOTIFY_HEADER_ERR, 70- BGP_NOTIFY_HEADER_BAD_MESLEN, 71- (unsigned char *)&nsize, 2); 72+ bgp_notify_io_invalid(peer, BGP_NOTIFY_HEADER_ERR, 73+ BGP_NOTIFY_HEADER_BAD_MESLEN, 74+ (unsigned char *)&nsize, 2); 75 return false; 76 } 77 78diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c 79index 7c92a8d9e..a5ce5a527 100644 80--- a/bgpd/bgp_packet.c 81+++ b/bgpd/bgp_packet.c 82@@ -736,8 +736,9 @@ static void bgp_write_notify(struct peer *peer) 83 * @param data Data portion 84 * @param datalen length of data portion 85 */ 86-void bgp_notify_send_with_data(struct peer *peer, uint8_t code, 87- uint8_t sub_code, uint8_t *data, size_t datalen) 88+static void bgp_notify_send_internal(struct peer *peer, uint8_t code, 89+ uint8_t sub_code, uint8_t *data, 90+ size_t datalen, bool use_curr) 91 { 92 struct stream *s; 93 94@@ -769,8 +770,11 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code, 95 * If possible, store last packet for debugging purposes. This check is 96 * in place because we are sometimes called with a doppelganger peer, 97 * who tends to have a plethora of fields nulled out. 98+ * 99+ * Some callers should not attempt this - the io pthread for example 100+ * should not touch internals of the peer struct. 101 */ 102- if (peer->curr) { 103+ if (use_curr && peer->curr) { 104 size_t packetsize = stream_get_endp(peer->curr); 105 assert(packetsize <= peer->max_packet_size); 106 memcpy(peer->last_reset_cause, peer->curr->data, packetsize); 107@@ -853,7 +857,27 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code, 108 */ 109 void bgp_notify_send(struct peer *peer, uint8_t code, uint8_t sub_code) 110 { 111- bgp_notify_send_with_data(peer, code, sub_code, NULL, 0); 112+ bgp_notify_send_internal(peer, code, sub_code, NULL, 0, true); 113+} 114+ 115+/* 116+ * Enqueue notification; called from the main pthread, peer object access is ok. 117+ */ 118+void bgp_notify_send_with_data(struct peer *peer, uint8_t code, 119+ uint8_t sub_code, uint8_t *data, size_t datalen) 120+{ 121+ bgp_notify_send_internal(peer, code, sub_code, data, datalen, true); 122+} 123+ 124+/* 125+ * For use by the io pthread, queueing a notification but avoiding access to 126+ * the peer object. 127+ */ 128+void bgp_notify_io_invalid(struct peer *peer, uint8_t code, uint8_t sub_code, 129+ uint8_t *data, size_t datalen) 130+{ 131+ /* Avoid touching the peer object */ 132+ bgp_notify_send_internal(peer, code, sub_code, data, datalen, false); 133 } 134 135 /* 136diff --git a/bgpd/bgp_packet.h b/bgpd/bgp_packet.h 137index 280d3ec17..898f88ff5 100644 138--- a/bgpd/bgp_packet.h 139+++ b/bgpd/bgp_packet.h 140@@ -62,6 +62,8 @@ extern void bgp_open_send(struct peer *); 141 extern void bgp_notify_send(struct peer *, uint8_t, uint8_t); 142 extern void bgp_notify_send_with_data(struct peer *, uint8_t, uint8_t, 143 uint8_t *, size_t); 144+void bgp_notify_io_invalid(struct peer *peer, uint8_t code, uint8_t sub_code, 145+ uint8_t *data, size_t datalen); 146 extern void bgp_route_refresh_send(struct peer *peer, afi_t afi, safi_t safi, 147 uint8_t orf_type, uint8_t when_to_refresh, 148 int remove, uint8_t subtype); 149-- 1502.25.1 151 152