On Thu, Feb 16, 2023 at 02:07:20AM +0100, Stefano Brivio wrote:

passt supports ranges of forwarded ports as well as 'all' for TCP and UDP, so it might be convenient to proceed if we fail to bind only some of the desired ports.

But if we fail to bind even a single port for a given specification, we're clearly, unexpectedly, conflicting with another network service. In that case, report failure and exit.

Reported-by: Yalan Zhang Signed-off-by: Stefano Brivio

Reviewed-by: David Gibson

--- conf.c | 47 ++++++++++++++++++++++++++++++++++------------- tcp.c | 25 ++++++++++++++++++------- tcp.h | 4 ++-- udp.c | 16 +++++++++++++--- udp.h | 4 ++-- 5 files changed, 69 insertions(+), 27 deletions(-)

diff --git a/conf.c b/conf.c index f175405..675d961 100644 --- a/conf.c +++ b/conf.c @@ -179,9 +179,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, { char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf; char buf[BUFSIZ], *spec, *ifname = NULL, *p; + bool exclude_only = true, bound_one = false; uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; sa_family_t af = AF_UNSPEC; - bool exclude_only = true;

if (!strcmp(optarg, "none")) { if (fwd->mode) @@ -215,12 +215,19 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, memset(fwd->map, 0xff, PORT_EPHEMERAL_MIN / 8);

for (i = 0; i < PORT_EPHEMERAL_MIN; i++) { - if (optname == 't') - tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i); - else if (optname == 'u') - udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i); + if (optname == 't') { + if (!tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i)) + bound_one = true; + } else if (optname == 'u') { + if (!udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, + i)) + bound_one = true; + } }

+ if (!bound_one) + goto bind_fail; + return; }

@@ -293,12 +300,18 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,

bitmap_set(fwd->map, i);

- if (optname == 't') - tcp_sock_init(c, af, addr, ifname, i); - else if (optname == 'u') - udp_sock_init(c, 0, af, addr, ifname, i); + if (optname == 't') { + if (!tcp_sock_init(c, af, addr, ifname, i)) + bound_one = true; + } else if (optname == 'u') { + if (!udp_sock_init(c, 0, af, addr, ifname, i)) + bound_one = true; + } }

+ if (!bound_one) + goto bind_fail; + return; }

@@ -339,13 +352,19 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg,

fwd->delta[i] = mapped_range.first - orig_range.first;

- if (optname == 't') - tcp_sock_init(c, af, addr, ifname, i); - else if (optname == 'u') - udp_sock_init(c, 0, af, addr, ifname, i); + if (optname == 't') { + if (!tcp_sock_init(c, af, addr, ifname, i)) + bound_one = true; + } else if (optname == 'u') { + if (udp_sock_init(c, 0, af, addr, ifname, i)) + bound_one = true; + } } } while ((p = next_chunk(p, ',')));

+ if (!bound_one) + goto bind_fail; + return; bad: die("Invalid port specifier %s", optarg); @@ -353,6 +372,8 @@ overlap: die("Overlapping port specifier %s", optarg); mode_conflict: die("Port forwarding mode '%s' conflicts with previous mode", optarg); +bind_fail: + die("Failed to bind any port for '-%c %s', exiting", optname, optarg); }

/** diff --git a/tcp.c b/tcp.c index f028e01..32ce1e9 100644 --- a/tcp.c +++ b/tcp.c @@ -2916,20 +2916,31 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port, * @addr: Pointer to address for binding, NULL if not configured * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order + * + * Return: 0 on (partial) success, -1 on (complete) failure */ -void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, - const char *ifname, in_port_t port) +int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, + const char *ifname, in_port_t port) { + int ret = 0; + if (af == AF_UNSPEC && c->ifi4 && c->ifi6) /* Attempt to get a dual stack socket */ if (tcp_sock_init_af(c, AF_UNSPEC, port, addr, ifname) >= 0) - return; + return 0;

/* Otherwise create a socket per IP version */ - if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) - tcp_sock_init_af(c, AF_INET, port, addr, ifname); - if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) - tcp_sock_init_af(c, AF_INET6, port, addr, ifname); + if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) { + if (tcp_sock_init_af(c, AF_INET, port, addr, ifname) < 0) + ret = -1; + } + + if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) { + if (tcp_sock_init_af(c, AF_INET6, port, addr, ifname) < 0) + ret = -1; + } + + return ret; }

/** diff --git a/tcp.h b/tcp.h index 236038f..5527c5b 100644 --- a/tcp.h +++ b/tcp.h @@ -17,8 +17,8 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now); int tcp_tap_handler(struct ctx *c, int af, const void *addr, const struct pool *p, const struct timespec *now); -void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, - const char *ifname, in_port_t port); +int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, + const char *ifname, in_port_t port); int tcp_init(struct ctx *c); void tcp_timer(struct ctx *c, const struct timespec *ts); void tcp_defer_handler(struct ctx *c); diff --git a/udp.c b/udp.c index 2e9b7e6..c913d27 100644 --- a/udp.c +++ b/udp.c @@ -955,12 +955,14 @@ int udp_tap_handler(struct ctx *c, int af, const void *addr, * @addr: Pointer to address for binding, NULL if not configured * @ifname: Name of interface to bind to, NULL if not configured * @port: Port, host order + * + * Return: 0 on (partial) success, -1 on (complete) failure */ -void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, const char *ifname, in_port_t port) +int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, + const void *addr, const char *ifname, in_port_t port) { union udp_epoll_ref uref = { .u32 = 0 }; - int s; + int s, ret = 0;

if (ns) { uref.udp.port = (in_port_t)(port + @@ -989,6 +991,9 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, ifname, port, uref.u32); udp_splice_ns[V4][port].sock = s; } + + if (s < 0) + ret = -1; }

if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) { @@ -1009,7 +1014,12 @@ void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, ifname, port, uref.u32); udp_splice_ns[V6][port].sock = s; } + + if (s < 0) + ret = -1; } + + return ret; }

/** diff --git a/udp.h b/udp.h index 68082ea..0e81be8 100644 --- a/udp.h +++ b/udp.h @@ -12,8 +12,8 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, const struct timespec *now); int udp_tap_handler(struct ctx *c, int af, const void *addr, const struct pool *p, const struct timespec *now); -void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, - const void *addr, const char *ifname, in_port_t port); +int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, + const void *addr, const char *ifname, in_port_t port); int udp_init(struct ctx *c); void udp_timer(struct ctx *c, const struct timespec *ts); void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s,

-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson