Bug 80 - UDP flow does not come back in ns when using -U option with port forwarding
Summary: UDP flow does not come back in ns when using -U option with port forwarding
Status: RESOLVED FIXED
Alias: None
Product: passt
Classification: Unclassified
Component: UDP (show other bugs)
Version: unspecified
Hardware: All Linux
: Highest blocker
Assignee: David Gibson
URL:
Depends on:
Blocks:
 
Reported: 2024-02-14 13:15 UTC by Laurent Jacquot
Modified: 2024-02-21 10:21 UTC (History)
2 users (show)

See Also:


Attachments

Description Laurent Jacquot 2024-02-14 13:15:53 UTC
Using current fedora 39 package passt-0^20231230.gf091893-1.fc39.x86_64

Step to reproduce:

1/ create a dns server on the host listening on the 5400 udp ort:
root@jack:~#dnsmasq -C /dev/null -h -k -d -q -R --server=2a01:e0c:1:1599::22 -p 5400

2/ using a non root id, create a pasta namespace forwarding port ns 5500 to host port 5400:
pod@jack:~$pasta --debug  --config-net -a 2a01:e0a:1f1:9700::f14 -o 2a01:e0a:1f1:9700::14 -a 192.168.23.14 -n 32  -U 5500:5400

3/ launch a dig query in the namespace:
podt@jack:~#dig  -p 5500 A free.fr @::1 +short
;; communications error to ::1#5500: timed out


What is working:
1/ same as before
root@jack:~#dnsmasq -C /dev/null -h -k -d -q -R --server=2a01:e0c:1:1599::22 -p 5400

2/ use 5200 instead of 5500 in the port forwarding options:
pod@jack:~$pasta --debug  --config-net -a 2a01:e0a:1f1:9700::f14 -o 2a01:e0a:1f1:9700::14 -a 192.168.23.14 -n 32  -U 5200:5400

3/ launch a dig query in the namespace:
pod@jack:~#dig  -p 5200 A free.fr @::1 +short
212.27.48.10

So the problem is -U nsport:hostport when nsport > hostport (works when equal)

the fix proposed in https://bugs.passt.top/show_bug.cgi?id=79#c25 does not fix the problem, it makes de -U 5500:5400 work but -U 5200:5400 fail
Comment 1 Laurent Jacquot 2024-02-18 18:48:16 UTC
Hello
the udp_portmap_clear() calculates the reverse port translation using unsigned ints with wraparound.
Unfortunately, the index of fwd->rdelta[] is incorrectly casted, and when starting pasta with -U nsport:hostport and nsport > hostport, the (int_port_t)i + delta is well over the size of fwd->rdelta[] (which is NUM_PORTS).

the reverse translation fails, but there also might be other implications as arbitrary memory is overwritten.

this patch works for me:

diff -ur ../passt.orig/udp.c ./udp.c
--- ../passt.orig/udp.c	2023-12-30 11:45:27.000000000 +0100
+++ ./udp.c	2024-02-18 19:32:59.869428863 +0100
@@ -265,7 +265,7 @@
 		in_port_t delta = fwd->f.delta[i];
 
 		if (delta)
-			fwd->rdelta[(in_port_t)i + delta] = NUM_PORTS - delta;
+			fwd->rdelta[(in_port_t)(i + delta)] = NUM_PORTS - delta;
 	}
 }
Comment 2 David Gibson 2024-02-20 00:52:22 UTC
Laurent,

Thanks for the report.  I hadn't previously been able to reproduce this problem when you mentioned it in bug 79 - turns out I'd only tried nsport < hostport.  This is a lot nastier than I'd first realized, since there's a buffer overrun, not just a failure to forward.

Thanks also for tracking down the problem.  Turns out that piece of code was actually intentional - but based on forgetting some of the minutiae of C's promotion rules.  I'd assumed that adding two things of type (in_port_t) would result in (in_port_t) arithmetic (unsigned, 16-bit).  But, no, they're promoted to int, hence the overrun described.

I've taken your patch and made a couple of small tweaks for clarity.  Will post as soon as the test run completes.
Comment 3 David Gibson 2024-02-21 00:39:57 UTC
Laurent,

The fix is now merged, it would be great if you could double check it.
Comment 4 Laurent Jacquot 2024-02-21 10:21:33 UTC
Hello,
It works for me:

root@jack:~#dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-0ed46a2d78

installs passt-0^20240220.g1e6f92b-1.fc39.x86_64.rpm

then 
pod@jack:~$pasta --debug  --config-net -a 2a01:e0a:1f1:9700::f14 -o 2a01:e0a:1f1:9700::14 -a 192.168.23.14 -n 32  -U 5400:5200

pod@jack:~$dig -p 5400 A free.fr @::1 +short
212.27.48.10

I'm closing the bug.
Thanks!

Note You need to log in before you can comment on or make changes to this bug.