Fix race between outbound messages and peer disconnection 2023-10-peer-race-send-discon
authorMatt Corallo <git@bluematt.me>
Wed, 18 Oct 2023 15:22:26 +0000 (15:22 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 18 Oct 2023 19:35:00 +0000 (19:35 +0000)
Previously, outbound messages held in `process_events` could race
with peer disconnection, allowing a message intended for a peer
before disconnection to be sent to the same peer after
disconnection.

The fix is simple - hold the peers read lock while we fetch
pending messages from peers (as we disconnect with the write lock).

lightning/src/ln/peer_handler.rs

index 933e7ee6ba58ee67e46260d14164ab0d46588a10..8e91023b1b132f0665ef49e6fa5c6f132419fe59 100644 (file)
@@ -1870,15 +1870,13 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
                        let flush_read_disabled = self.gossip_processing_backlog_lifted.swap(false, Ordering::Relaxed);
 
                        let mut peers_to_disconnect = HashMap::new();
-                       let mut events_generated = self.message_handler.chan_handler.get_and_clear_pending_msg_events();
-                       events_generated.append(&mut self.message_handler.route_handler.get_and_clear_pending_msg_events());
 
                        {
-                               // TODO: There are some DoS attacks here where you can flood someone's outbound send
-                               // buffer by doing things like announcing channels on another node. We should be willing to
-                               // drop optional-ish messages when send buffers get full!
-
                                let peers_lock = self.peers.read().unwrap();
+
+                               let mut events_generated = self.message_handler.chan_handler.get_and_clear_pending_msg_events();
+                               events_generated.append(&mut self.message_handler.route_handler.get_and_clear_pending_msg_events());
+
                                let peers = &*peers_lock;
                                macro_rules! get_peer_for_forwarding {
                                        ($node_id: expr) => {