This fixes bug 79 and makes another clean up in the same area. Link: https://bugs.passt.top/show_bug.cgi?id=79 David Gibson (2): udp: Don't prematurely (and incorrectly) set up automatic inbound forwards udp: udp_sock_init_ns() partially duplicats udp_port_rebind_outbound() udp.c | 90 +++++++++++++++++------------------------------------------ 1 file changed, 25 insertions(+), 65 deletions(-) -- 2.43.0
For automated inbound port forwarding in pasta mode we scan bound ports within the guest namespace via /proc and bind matching ports on the host to listen for packets. For UDP this is usually handled by udp_timer() which calls port_fwd_scan_udp() followed by udp_port_rebind(). However there's one initial scan before the the UDP timer is started: we call port_fwd_scan_udp() from port_fwd_init(), and actually bind the resulting ports in udp_sock_init_init() called from udp_init(). Unfortunately, the version in udp_sock_init_init() isn't correct. It unconditionally opens a new socket for every forwarded port, even if a socket has already been explicit created with the -u option. If the explicitly forwarded ports have particular configuration (such as a specific bound address address, or one implied by the -o option) those will not be replicated in the new socket. We essentially leak the original correctly configured socket, replacing it with one which might not be right. We could make udp_sock_init_init() use udp_port_rebind() to get that right, but there's actually no point doing so: * The initial bind was introduced by ccf6d2a7b48d ("udp: Actually bind detected namespace ports in init namespace") at which time we didn't periodically scan for bound UDP ports. Periodic scanning was introduced in 457ff122e ("udp,pasta: Periodically scan for ports to automatically forward") making the bind from udp_init() redundant. * At the time of udp_init(), programs in the guest namespace are likely not to have started yet (unless attaching a pre-existing namespace) so there's likely not anything to scan for anyway. So, simply remove the initial, broken socket create/bind, allowing automatic port forwards to be created the first time udp_timer() runs. Reported-by: Laurent Jacquot <jk(a)lutty.net> Suggested-by: Laurent Jacquot <jk(a)lutty.net> Link: https://bugs.passt.top/show_bug.cgi?id=79 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- udp.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/udp.c b/udp.c index b5b8f8a7..b240185e 100644 --- a/udp.c +++ b/udp.c @@ -1041,22 +1041,6 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, return r4 < 0 ? r4 : r6; } -/** - * udp_sock_init_init() - Bind sockets in init namespace for inbound connections - * @c: Execution context - */ -static void udp_sock_init_init(const struct ctx *c) -{ - unsigned dst; - - for (dst = 0; dst < NUM_PORTS; dst++) { - if (!bitmap_isset(c->udp.fwd_in.f.map, dst)) - continue; - - udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, dst); - } -} - /** * udp_sock_init_ns() - Bind sockets in namespace for outbound connections * @arg: Execution context @@ -1125,7 +1109,6 @@ int udp_init(struct ctx *c) if (c->mode == MODE_PASTA) { udp_splice_iov_init(); - udp_sock_init_init(c); NS_CALL(udp_sock_init_ns, c); } -- 2.43.0
Usually automatically forwarded UDP outbound ports are set up by udp_port_rebind_outbound() called from udp_timer(). However, the very first time they're created and bound is by udp_sock_init_ns() called from udp_init(). udp_sock_init_ns() is essentially an unnecessary cut down version of udp_port_rebind_outbound(), so we can jusat remove it. Doing so does require moving udp_init() below udp_port_rebind_outbound()'s definition. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- udp.c | 73 ++++++++++++++++++++--------------------------------------- 1 file changed, 25 insertions(+), 48 deletions(-) diff --git a/udp.c b/udp.c index b240185e..933f24b8 100644 --- a/udp.c +++ b/udp.c @@ -1041,29 +1041,6 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, return r4 < 0 ? r4 : r6; } -/** - * udp_sock_init_ns() - Bind sockets in namespace for outbound connections - * @arg: Execution context - * - * Return: 0 - */ -int udp_sock_init_ns(void *arg) -{ - const struct ctx *c = (const struct ctx *)arg; - unsigned dst; - - ns_enter(c); - - for (dst = 0; dst < NUM_PORTS; dst++) { - if (!bitmap_isset(c->udp.fwd_out.f.map, dst)) - continue; - - udp_sock_init(c, 1, AF_UNSPEC, NULL, NULL, dst); - } - - return 0; -} - /** * udp_splice_iov_init() - Set up buffers and descriptors for recvmmsg/sendmmsg */ @@ -1090,31 +1067,6 @@ static void udp_splice_iov_init(void) } } -/** - * udp_init() - Initialise per-socket data, and sockets in namespace - * @c: Execution context - * - * Return: 0 - */ -int udp_init(struct ctx *c) -{ - if (c->ifi4) - udp_sock4_iov_init(c); - - if (c->ifi6) - udp_sock6_iov_init(c); - - udp_invert_portmap(&c->udp.fwd_in); - udp_invert_portmap(&c->udp.fwd_out); - - if (c->mode == MODE_PASTA) { - udp_splice_iov_init(); - NS_CALL(udp_sock_init_ns, c); - } - - return 0; -} - /** * udp_timer_one() - Handler for timed events on one port * @c: Execution context @@ -1272,3 +1224,28 @@ v6: goto v6; } } + +/** + * udp_init() - Initialise per-socket data, and sockets in namespace + * @c: Execution context + * + * Return: 0 + */ +int udp_init(struct ctx *c) +{ + if (c->ifi4) + udp_sock4_iov_init(c); + + if (c->ifi6) + udp_sock6_iov_init(c); + + udp_invert_portmap(&c->udp.fwd_in); + udp_invert_portmap(&c->udp.fwd_out); + + if (c->mode == MODE_PASTA) { + udp_splice_iov_init(); + NS_CALL(udp_port_rebind_outbound, c); + } + + return 0; +} -- 2.43.0
On Mon, 12 Feb 2024 17:06:56 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:This fixes bug 79 and makes another clean up in the same area. Link: https://bugs.passt.top/show_bug.cgi?id=79 David Gibson (2): udp: Don't prematurely (and incorrectly) set up automatic inbound forwards udp: udp_sock_init_ns() partially duplicats udp_port_rebind_outbound() udp.c | 90 +++++++++++++++++------------------------------------------ 1 file changed, 25 insertions(+), 65 deletions(-)Applied. -- Stefano