fwd: Exclude ports based on prior mapping state

With auto port-forwarding modes we scan for listening ports on the host
and/or guest and create forwardings for them.  To avoid circular forwarding
we need to exclude our own listening ports.  We do this by masking out
the forwarding map for one direction from the other.

Since 1bc7d5485c10, some of our scans take place while the forward maps are
out of sync with what our actual listening ports are though: the map
represents what we intend to forward shortly, rather than what we have
open sockets for right now.

What we have sockets for right now is what matters for the purposes of
excluding from the scan, though, so that was incorrect.  So, restore
correct behaviour by saving the map of ports to exclude before we start
updating any of the forwarding maps with new scans.  This allows us to
keep all the scans separate from all the rebinds, and therefore several
minor cleanups that permitted.

As a bonus, pre-creating the exclusion bitmaps this way should make this
code easier to adapt as we change the forwarding data structures to allow
more flexible configuration.

Fixes: 1bc7d5485c10 ("fwd: Consolidate scans (not rebinds) in fwd.c")
Link: https://bugs.passt.top/show_bug.cgi?id=176
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

2 files changed, 20 insertions, 22 deletions

diff --git a/fwd.c b/fwd.c
index 7b6c40f..c7a880e 100644
--- a/fwd.c
+++ b/fwd.c
@@ -358,10 +358,9 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map)
 /**
  * fwd_scan_ports_tcp() - Scan /proc to update TCP forwarding map
  * @fwd:	Forwarding information to update
- * @rev:	Forwarding information for the reverse direction
+ * @exclude:	Ports to _not_ forward
  */
-static void fwd_scan_ports_tcp(struct fwd_ports *fwd,
-			       const struct fwd_ports *rev)
+static void fwd_scan_ports_tcp(struct fwd_ports *fwd, const uint8_t *exclude)
 {
 	if (fwd->mode != FWD_AUTO)
 		return;
@@ -369,20 +368,18 @@ static void fwd_scan_ports_tcp(struct fwd_ports *fwd,
 	memset(fwd->map, 0, PORT_BITMAP_SIZE);
 	procfs_scan_listen(fwd->scan4, TCP_LISTEN, fwd->map);
 	procfs_scan_listen(fwd->scan6, TCP_LISTEN, fwd->map);
-	bitmap_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
+	bitmap_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, exclude);
 }
 
 /**
  * fwd_scan_ports_udp() - Scan /proc to update UDP forwarding map
  * @fwd:	Forwarding information to update
- * @rev:	Forwarding information for the reverse direction
  * @tcp_fwd:	Corresponding TCP forwarding information
- * @tcp_rev:	TCP forwarding information for the reverse direction
+ * @exclude:	Ports to _not_ forward
  */
 static void fwd_scan_ports_udp(struct fwd_ports *fwd,
-			       const struct fwd_ports *rev,
 			       const struct fwd_ports *tcp_fwd,
-			       const struct fwd_ports *tcp_rev)
+			       const uint8_t *exclude)
 {
 	if (fwd->mode != FWD_AUTO)
 		return;
@@ -399,13 +396,7 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
 	procfs_scan_listen(tcp_fwd->scan4, TCP_LISTEN, fwd->map);
 	procfs_scan_listen(tcp_fwd->scan6, TCP_LISTEN, fwd->map);
 
-	/* This means we need to skip numbers of TCP ports bound on the other
-	 * side, too. Otherwise, we would detect corresponding UDP ports as
-	 * bound and try to forward them from the opposite side, but it's
-	 * already us handling them.
-	 */
-	bitmap_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, rev->map);
-	bitmap_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, tcp_rev->map);
+	bitmap_and_not(fwd->map, PORT_BITMAP_SIZE, fwd->map, exclude);
 }
 
 /**
@@ -414,12 +405,20 @@ static void fwd_scan_ports_udp(struct fwd_ports *fwd,
  */
 static void fwd_scan_ports(struct ctx *c)
 {
-	fwd_scan_ports_tcp(&c->tcp.fwd_out, &c->tcp.fwd_in);
-	fwd_scan_ports_tcp(&c->tcp.fwd_in, &c->tcp.fwd_out);
-	fwd_scan_ports_udp(&c->udp.fwd_out, &c->udp.fwd_in,
-			   &c->tcp.fwd_out, &c->tcp.fwd_in);
-	fwd_scan_ports_udp(&c->udp.fwd_in, &c->udp.fwd_out,
-			   &c->tcp.fwd_in, &c->tcp.fwd_out);
+	uint8_t excl_tcp_out[PORT_BITMAP_SIZE], excl_udp_out[PORT_BITMAP_SIZE];
+	uint8_t excl_tcp_in[PORT_BITMAP_SIZE], excl_udp_in[PORT_BITMAP_SIZE];
+
+	memcpy(excl_tcp_out, c->tcp.fwd_in.map, sizeof(excl_tcp_out));
+	memcpy(excl_tcp_in, c->tcp.fwd_out.map, sizeof(excl_tcp_in));
+	bitmap_or(excl_udp_out, PORT_BITMAP_SIZE,
+		  c->udp.fwd_in.map, c->tcp.fwd_in.map);
+	bitmap_or(excl_udp_in, PORT_BITMAP_SIZE,
+		  c->udp.fwd_out.map, c->tcp.fwd_out.map);
+
+	fwd_scan_ports_tcp(&c->tcp.fwd_out, excl_tcp_out);
+	fwd_scan_ports_tcp(&c->tcp.fwd_in, excl_tcp_in);
+	fwd_scan_ports_udp(&c->udp.fwd_out, &c->tcp.fwd_out, excl_udp_out);
+	fwd_scan_ports_udp(&c->udp.fwd_in, &c->tcp.fwd_in, excl_udp_in);
 }
 
 /**
diff --git a/util.c b/util.c
index ab23463..7944a49 100644
--- a/util.c
+++ b/util.c
@@ -338,7 +338,6 @@ bool bitmap_isset(const uint8_t *map, unsigned bit)
  * @a:		First operand
  * @b:		Second operand
  */
-/* cppcheck-suppress unusedFunction */
 void bitmap_or(uint8_t *dst, size_t size, const uint8_t *a, const uint8_t *b)
 {
 	unsigned long *dw = (unsigned long *)dst;