From 20e3454c267534ef727bfdd0169794d521405682 Mon Sep 17 00:00:00 2001 From: Jonas K Danielsson Date: Wed, 29 May 2024 06:33:50 +0200 Subject: [PATCH] udpsrc: Disable allocated port reuse for unicast The `reuse` property end up setting the SO_REUSEADDR socket option for the UDP socket. This setting have surprising effects. On Linux systems the man page (`socket(7)`) states: ``` SO_REUSEADDR Indicates that the rules used in validating addresses supplied in a bind(2) call should allow reuse of local addresses. For AF_INET sockets this means that a socket may bind, except when there is an active listening socket bound to the address. ``` But since UDP does not listen this ends up meaning that when an ephemeral port is allocated (setting the `port` to `0`) the kernel is free to reuse any other UDP port that has `SO_REUSEADDR` set. Tests checking the likelyhood of port conflict when using multiple `udpsrc` shows port conflicts starting to occur after ~100-300 udpsrc with port allocation enabled. See issue #3411 for more details. Changing the default value of a property is not a small thing we risk breaking application that rely on the current default value. But since the effects of having `reuse` default `TRUE` on can also have damaging and hard-to-debug consequences, it might be worth to consider. Having `SO_REUSEADDR` enabled for multicast, might have some use cases but for unicast, with dynamic port allocation, it does not make sense. When not using an multicast address we will disable port reuse if the `port` property is set to 0 (=allocate) and warn the user that we did so. Closes #3411 Part-of: --- subprojects/gst-plugins-good/gst/udp/gstudpsrc.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/subprojects/gst-plugins-good/gst/udp/gstudpsrc.c b/subprojects/gst-plugins-good/gst/udp/gstudpsrc.c index a5a35817a3..96cdcf778d 100644 --- a/subprojects/gst-plugins-good/gst/udp/gstudpsrc.c +++ b/subprojects/gst-plugins-good/gst/udp/gstudpsrc.c @@ -1589,6 +1589,7 @@ gst_udpsrc_open (GstUDPSrc * src) GInetAddress *addr, *bind_addr; GSocketAddress *bind_saddr; GError *err = NULL; + gboolean allow_port_reuse = src->reuse; if (src->source_addrs) { g_list_free_full (src->source_addrs, (GDestroyNotify) g_object_unref); @@ -1632,7 +1633,15 @@ gst_udpsrc_open (GstUDPSrc * src) bind_saddr = g_inet_socket_address_new (bind_addr, src->port); g_object_unref (bind_addr); - if (!g_socket_bind (src->used_socket, bind_saddr, src->reuse, &err)) { + + if (allow_port_reuse && src->port == 0 + && !g_inet_address_get_is_multicast (addr)) { + GST_WARNING_OBJECT (src, + "Disabling port reuse for dynamically allocated port to avoid potential conflicts"); + allow_port_reuse = FALSE; + } + + if (!g_socket_bind (src->used_socket, bind_saddr, allow_port_reuse, &err)) { GST_ERROR_OBJECT (src, "%s: error binding to %s:%d", err->message, src->address, src->port); goto bind_error;