Fix connection locking in firewall packet handler

network/connection.go

@@ -580,10 +580,7 @@ func (conn *Connection) SetFirewallHandler(handler FirewallHandler) {
 		conn.pktQueue = make(chan packet.Packet, 1000)
 
 		// start handling
-		module.StartWorker("packet handler", func(ctx context.Context) error {
+		module.StartWorker("packet handler", conn.packetHandlerWorker)
-			conn.packetHandler()
-			return nil
-		})
 	}
 	conn.firewallHandler = handler
 }
@@ -608,35 +605,46 @@ func (conn *Connection) HandlePacket(pkt packet.Packet) {
 	}
 }
 
-// packetHandler sequentially handles queued packets.
+// packetHandlerWorker sequentially handles queued packets.
-func (conn *Connection) packetHandler() {
+func (conn *Connection) packetHandlerWorker(ctx context.Context) error {
-	for pkt := range conn.pktQueue {
+	for {
-		if pkt == nil {
+		select {
-			return
+		case pkt := <-conn.pktQueue:
+			if pkt == nil {
+				return nil
+			}
+			packetHandlerHandleConn(conn, pkt)
+
+		case <-ctx.Done():
+			conn.Lock()
+			defer conn.Unlock()
+			conn.firewallHandler = nil
+			return nil
 		}
-		// get handler
+	}
-		conn.Lock()
+}
 
-		// execute handler or verdict
+func packetHandlerHandleConn(conn *Connection, pkt packet.Packet) {
-		if conn.firewallHandler != nil {
+	conn.Lock()
-			conn.firewallHandler(conn, pkt)
+	defer conn.Unlock()
-		} else {
-			defaultFirewallHandler(conn, pkt)
-		}
 
-		// log verdict
+	// Handle packet with appropriate handler.
-		log.Tracer(pkt.Ctx()).Infof("filter: connection %s %s: %s", conn, conn.Verdict.Verb(), conn.Reason.Msg)
+	if conn.firewallHandler != nil {
-		// submit trace logs
+		conn.firewallHandler(conn, pkt)
-		log.Tracer(pkt.Ctx()).Submit()
+	} else {
+		defaultFirewallHandler(conn, pkt)
+	}
 
-		// save does not touch any changing data
+	// Log verdict.
-		// must not be locked, will deadlock with cleaner functions
+	log.Tracer(pkt.Ctx()).Infof("filter: connection %s %s: %s", conn, conn.Verdict.Verb(), conn.Reason.Msg)
-		if conn.saveWhenFinished {
+	// Submit trace logs.
-			conn.saveWhenFinished = false
+	log.Tracer(pkt.Ctx()).Submit()
-			conn.Save()
-		}
 
-		conn.Unlock()
+	// Save() itself does not touch any changing data.
+	// Must not be locked - would deadlock with cleaner functions.
+	if conn.saveWhenFinished {
+		conn.saveWhenFinished = false
+		conn.Save()
 	}
 }