From: dlg <dlg@openbsd.org>
Date: Fri, 25 Jun 2021 23:48:30 +0000 (+0000)
Subject: let pfsync_request_update actually retry when it overfills a packet.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=7845b46b2ba574e7f66bc4888cb6171c09dd88f3;p=openbsd

let pfsync_request_update actually retry when it overfills a packet.

a continue in the middle of a do { } while (0) loop is effectively
a break, it doesnt restart the loop.

without the retry, the code leaked update messages which in turn
made pool_destroy in pfsync destroy trip over a kassert cos items
were still out.

found by and fix tested by hrvoje popovski
ok sashan@
---

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index eaaaa26f19d..744a7ca9dbc 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.295 2021/06/23 06:53:51 dlg Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.296 2021/06/25 23:48:30 dlg Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -2232,6 +2232,7 @@ pfsync_request_update(u_int32_t creatorid, u_int64_t id)
 	struct pfsync_softc *sc = pfsyncif;
 	struct pfsync_upd_req_item *item;
 	size_t nlen, sc_len;
+	int retry;
 
 	/*
 	 * this code does nothing to prevent multiple update requests for the
@@ -2247,7 +2248,7 @@ pfsync_request_update(u_int32_t creatorid, u_int64_t id)
 	item->ur_msg.id = id;
 	item->ur_msg.creatorid = creatorid;
 
-	do {
+	for (;;) {
 		mtx_enter(&sc->sc_upd_req_mtx);
 
 		nlen = sizeof(struct pfsync_upd_req);
@@ -2255,16 +2256,19 @@ pfsync_request_update(u_int32_t creatorid, u_int64_t id)
 			nlen += sizeof(struct pfsync_subheader);
 
 		sc_len = atomic_add_long_nv(&sc->sc_len, nlen);
-		if (sc_len > sc->sc_if.if_mtu) {
+		retry = (sc_len > sc->sc_if.if_mtu);
+		if (retry)
 			atomic_sub_long(&sc->sc_len, nlen);
-			mtx_leave(&sc->sc_upd_req_mtx);
-			pfsync_sendout();
-			continue;
-		}
+		else
+			TAILQ_INSERT_TAIL(&sc->sc_upd_req_list, item, ur_entry);
 
-		TAILQ_INSERT_TAIL(&sc->sc_upd_req_list, item, ur_entry);
 		mtx_leave(&sc->sc_upd_req_mtx);
-	} while (0);
+
+		if (!retry)
+			break;
+
+		pfsync_sendout();
+	}
 
 	schednetisr(NETISR_PFSYNC);
 }