From d5b80ccc72ed36367ac327748be66323c858ad5d Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sat, 24 Sep 2022 19:08:22 +1000 Subject: Fix widespread off-by-one error dealing with port numbers Port numbers (for both TCP and UDP) are 16-bit, and so fit exactly into a 'short'. USHRT_MAX is therefore the maximum port number and this is widely used in the code. Unfortunately, a lot of those places don't actually want the maximum port number (USHRT_MAX == 65535), they want the total number of ports (65536). This leads to a number of potentially nasty consequences: * We have buffer overruns on the port_fwd::delta array if we try to use port 65535 * We have similar potential overruns for the tcp_sock_* arrays * Interestingly udp_act had the correct size, but we can calculate it in a more direct manner * We have a logical overrun of the ports bitmap as well, although it will just use an unused bit in the last byte so isnt harmful * Many loops don't consider port 65535 (which does mitigate some but not all of the buffer overruns above) * In udp_invert_portmap() we incorrectly compute the reverse port translation for return packets Correct all these by using a new NUM_PORTS defined explicitly for this purpose. Signed-off-by: David Gibson --- udp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'udp.c') diff --git a/udp.c b/udp.c index 27c3aa3..bd3dc72 100644 --- a/udp.c +++ b/udp.c @@ -164,8 +164,8 @@ struct udp_splice_port { }; /* Port tracking, arrays indexed by packet source port (host order) */ -static struct udp_tap_port udp_tap_map [IP_VERSIONS][USHRT_MAX]; -static struct udp_splice_port udp_splice_map [IP_VERSIONS][USHRT_MAX]; +static struct udp_tap_port udp_tap_map [IP_VERSIONS][NUM_PORTS]; +static struct udp_splice_port udp_splice_map [IP_VERSIONS][NUM_PORTS]; enum udp_act_type { UDP_ACT_TAP, @@ -175,7 +175,7 @@ enum udp_act_type { }; /* Activity-based aging for bindings */ -static uint8_t udp_act[IP_VERSIONS][UDP_ACT_TYPE_MAX][(USHRT_MAX + 1) / 8]; +static uint8_t udp_act[IP_VERSIONS][UDP_ACT_TYPE_MAX][DIV_ROUND_UP(NUM_PORTS, 8)]; /* Static buffers */ @@ -271,7 +271,7 @@ static void udp_invert_portmap(struct udp_port_fwd *fwd) in_port_t delta = fwd->f.delta[i]; if (delta) - fwd->rdelta[(in_port_t)i + delta] = USHRT_MAX - delta; + fwd->rdelta[(in_port_t)i + delta] = NUM_PORTS - delta; } } @@ -1198,7 +1198,7 @@ int udp_sock_init_ns(void *arg) if (ns_enter(c)) return 0; - for (dst = 0; dst < USHRT_MAX; dst++) { + for (dst = 0; dst < NUM_PORTS; dst++) { if (!bitmap_isset(c->udp.fwd_out.f.map, dst)) continue; -- cgit v1.2.3