bgpd: Don't try to recursively hold peer io mutex

BGP was modified in a0b937de428e14e869b8541f0b7810113d619c2e
to grab the peer->io_mtx before validating the header to ensure
that the input Queue was not being modified by anyone else at that
moment in time.  Unfortunately validate_header can detect a problem
and attempt to relock the mutex, which deadlocks.  This deadlock in
the bgp_io pthread is the lone deadlock at first, eventually though
bgp attempts to write another packet to the peer( say when the
it's time to send the next packet ) and the main pthread of bgpd
becomes deadlocked and then the whole bgpd process is stuck at that
point in time leaving us dead in the water.

The point of locking the mutex earlier was to ensure that the input
Queue wasn't being modified by anyone else, (Say reading off it )
as that we wanted to ensure that we don't hold more packets then necessary.

Let's grab the mutex long enough to look at the input Q size, this
ensure that we have room and then we can validate_header and do the right
thing from there.  We'll need to lock the mutex when we actually move it
into the input Q as well.

Fixes: #12725
Signed-off-by: Donald Sharp <sharpd@nvidia.com>

1 files changed, 7 insertions, 6 deletions

diff --git a/bgpd/bgp_io.c b/bgpd/bgp_io.c
index 49ae9816a..f9dc64af4 100644
--- a/bgpd/bgp_io.c
+++ b/bgpd/bgp_io.c
@@ -173,12 +173,11 @@ static int read_ibuf_work(struct peer *peer)
 	uint16_t pktsize = 0;
 	struct stream *pkt;
 
-	/* Hold the I/O lock, we might not have space on the InQ */
-	frr_mutex_lock_autounlock(&peer->io_mtx);
 	/* ============================================== */
-
-	if (peer->ibuf->count >= bm->inq_limit)
-		return -ENOMEM;
+	frr_with_mutex (&peer->io_mtx) {
+		if (peer->ibuf->count >= bm->inq_limit)
+			return -ENOMEM;
+	}
 
 	/* check that we have enough data for a header */
 	if (ringbuf_remain(ibw) < BGP_HEADER_SIZE)
@@ -211,7 +210,9 @@ static int read_ibuf_work(struct peer *peer)
 	stream_set_endp(pkt, pktsize);
 
 	frrtrace(2, frr_bgp, packet_read, peer, pkt);
-	stream_fifo_push(peer->ibuf, pkt);
+	frr_with_mutex (&peer->io_mtx) {
+		stream_fifo_push(peer->ibuf, pkt);
+	}
 
 	return pktsize;
 }