Merge branch 'misc-fixes' into 'master'

Various fixes to the RTP module and one SPA fix

See merge request pipewire/pipewire!2770

spa/include/spa/pod/body.h

@@ -111,8 +111,15 @@ SPA_API_POD_BODY int spa_pod_choice_n_values(uint32_t choice_type, uint32_t *min
 SPA_API_POD_BODY int spa_pod_body_from_data(void *data, size_t maxsize, off_t offset, size_t size,
 		struct spa_pod *pod, const void **body)
 {
-	if (offset < 0 || offset > (int64_t)UINT32_MAX)
+	if (offset < 0)
 		return -EINVAL;
+	/* On 32-bit platforms, off_t is a signed 32-bit type (since it tracks pointer
+	 * width), and consequently can never exceed UINT32_MAX. Skip the upper-bound
+	 * check on 32-bit platforms to avoid a compiler warning. */
+#if __SIZEOF_POINTER__ > 4
+	if (offset > (int64_t)UINT32_MAX)
+		return -EINVAL;
+#endif
 	if (size < sizeof(struct spa_pod) ||
 	    size > maxsize ||
 	    maxsize - size < (uint32_t)offset)

src/modules/module-rtp-sap.c

@@ -1235,8 +1235,11 @@ static struct session *session_new_announce(struct impl *impl, struct node *node
 			sess->has_sdp = true;
 	}
 
+	pw_log_debug("sending out initial SAP");
 	send_sap(impl, sess, 0);
 
+	pw_log_debug("new announcement session up and running");
+
 	return sess;
 
 error_free:
@@ -1835,6 +1838,7 @@ static int start_sap(struct impl *impl)
 			pw_timer_queue_add(impl->timer_queue, &impl->start_sap_retry_timer,
 					NULL, 1 * SPA_NSEC_PER_SEC,
 					on_start_sap_retry_timer_event, impl);
+			pw_log_info("starting SAP retry timer");
 
 			/* It is important to return 0 in this case. Otherwise, the nonzero return
 			 * value will later be propagated through the core as an error. */
@@ -2005,8 +2009,10 @@ static void impl_destroy(struct impl *impl)
 	pw_timer_queue_cancel(&impl->sap_send_timer);
 	pw_timer_queue_cancel(&impl->start_sap_retry_timer);
 	pw_timer_queue_cancel(&impl->igmp_recovery.timer);
-	if (impl->sap_source)
+	if (impl->sap_source) {
+		pw_log_info("destroying SAP source");
 		pw_loop_destroy_source(impl->loop, impl->sap_source);
+	}
 
 	if (impl->sap_fd != -1)
 		close(impl->sap_fd);

src/modules/module-rtp-source.c

@@ -246,6 +246,9 @@ struct impl {
 	socklen_t src_len;
 	struct spa_source *source;
 
+	bool is_multicast;
+	bool filter_by_address;
+
 	uint8_t *buffer;
 	size_t buffer_size;
 
@@ -300,14 +303,41 @@ on_rtp_io(void *data, int fd, uint32_t mask)
 	ssize_t len;
 	int suppressed;
 	uint64_t current_time;
+	struct sockaddr_storage recvaddr;
+	socklen_t recvaddr_len = sizeof(recvaddr);
 
 	current_time = get_time_ns(impl);
 
 	if (mask & SPA_IO_IN) {
-
+		if ((len = recvfrom(fd, impl->buffer, impl->buffer_size, 0, (struct sockaddr *)(&recvaddr), &recvaddr_len)) < 0)
-		if ((len = recv(fd, impl->buffer, impl->buffer_size, 0)) < 0)
 			goto receive_error;
 
+		/* Filter the packets to exclude those with source addresses
+		 * that do not match the expected one. Only used with unicast.
+		 * (The bind() call in make_socket takes care of only
+		 * receiving packets that target the specified port.) */
+		if (impl->filter_by_address && !pw_net_are_addresses_equal(&recvaddr, &(impl->src_addr), false)) {
+			/* In the IPv6 case, pw_net_get_ip() produces output formatted
+			 * as "<IPv6 address>%<network interface name>". Both constants
+			 * INET6_ADDRSTRLEN and IFNAMSIZ include the null terminator
+			 * in their respective length values. This works out well for
+			 * the formatted output, since this ensures there is one extra
+			 * character for the % delimiter and another extra character
+			 * for the null terminator of the entire string.
+			 *
+			 * (In the IPv4 case, pw_net_get_ip() just outputs the address.) */
+			char address_str[INET6_ADDRSTRLEN + IFNAMSIZ];
+			int res;
+
+			res = pw_net_get_ip(&recvaddr, address_str, sizeof(address_str), NULL, NULL);
+			if (SPA_LIKELY(res == 0))
+				pw_log_trace("Filtering out packet with mismatching address %s", address_str);
+			else
+				pw_log_warn("Filtering out packet with unrecognized address");
+
+			return;
+		}
+
 		if (len < 12)
 			goto short_packet;
 
@@ -474,12 +504,12 @@ finish:
 }
 
 static int make_socket(const struct sockaddr* sa, socklen_t salen, char *ifname,
-			struct igmp_recovery *igmp_recovery)
+			struct igmp_recovery *igmp_recovery, bool *is_multicast,
+			bool *filter_by_address)
 {
 	int af, fd, val, res;
 	struct ifreq req;
 	struct sockaddr_storage ba = *(struct sockaddr_storage *)sa;
-	bool do_connect = false;
 	char addr[128];
 
 	af = sa->sa_family;
@@ -522,12 +552,16 @@ static int make_socket(const struct sockaddr* sa, socklen_t salen, char *ifname,
 			pw_net_get_ip((struct sockaddr_storage*)sa, addr, sizeof(addr), NULL, NULL);
 			pw_log_info("join IPv4 group: %s", addr);
 			res = setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP, &mr4, sizeof(mr4));
+			*filter_by_address = false;
+			*is_multicast = true;
 		} else {
 			struct sockaddr_in *ba4 = (struct sockaddr_in*)&ba;
-			if (ba4->sin_addr.s_addr != INADDR_ANY) {
+			*filter_by_address = (ba4->sin_addr.s_addr != INADDR_ANY);
-				ba4->sin_addr.s_addr = INADDR_ANY;
+			*is_multicast = false;
-				do_connect = true;
+			/* Make sure the ANY address is always used. This is important
-			}
+			 * for the bind() call below - with unicast, it shall only filter
+			 * by port number (address filtering is done by recvfrom()). */
+			ba4->sin_addr.s_addr = INADDR_ANY;
 		}
 	} else if (af == AF_INET6) {
 		struct sockaddr_in6 *sa6 = (struct sockaddr_in6*)sa;
@@ -539,8 +573,15 @@ static int make_socket(const struct sockaddr* sa, socklen_t salen, char *ifname,
 			pw_net_get_ip((struct sockaddr_storage*)sa, addr, sizeof(addr), NULL, NULL);
 			pw_log_info("join IPv6 group: %s", addr);
 			res = setsockopt(fd, IPPROTO_IPV6, IPV6_JOIN_GROUP, &mr6, sizeof(mr6));
+			*filter_by_address = false;
+			*is_multicast = true;
 		} else {
 			struct sockaddr_in6 *ba6 = (struct sockaddr_in6*)&ba;
+			*filter_by_address = !IN6_IS_ADDR_UNSPECIFIED(&(ba6->sin6_addr.s6_addr));
+			*is_multicast = false;
+			/* Make sure the ANY address is always used. This is important
+			 * for the bind() call below - with unicast, it shall only filter
+			 * by port number (address filtering is done by recvfrom()). */
 			ba6->sin6_addr = in6addr_any;
 		}
 	} else {
@@ -569,13 +610,6 @@ static int make_socket(const struct sockaddr* sa, socklen_t salen, char *ifname,
 		pw_log_error("bind() failed: %m");
 		goto error;
 	}
-	if (do_connect) {
-		if (connect(fd, sa, salen) < 0) {
-			res = -errno;
-			pw_log_error("connect() failed: %m");
-			goto error;
-		}
-	}
 	return fd;
 error:
 	close(fd);
@@ -613,7 +647,9 @@ static void stream_open_connection(void *data, int *result)
 
 	if ((fd = make_socket((const struct sockaddr *)&impl->src_addr,
 					impl->src_len, impl->ifname,
-					&(impl->igmp_recovery))) < 0) {
+					&(impl->igmp_recovery),
+					&(impl->is_multicast),
+					&(impl->filter_by_address))) < 0) {
 		/* If make_socket() tries to create a socket and join to a multicast
 		 * group while the network interfaces are not ready yet to do so
 		 * (usually because a network manager component is still setting up
@@ -663,11 +699,13 @@ static void stream_open_connection(void *data, int *result)
 		goto finish;
 	}
 
-	if ((res = pw_timer_queue_add(impl->timer_queue, &impl->igmp_recovery.timer,
+	if (impl->is_multicast) {
-			NULL, impl->igmp_recovery.check_interval * SPA_NSEC_PER_SEC,
+		if ((res = pw_timer_queue_add(impl->timer_queue, &impl->igmp_recovery.timer,
-			on_igmp_recovery_timer_event, impl)) < 0) {
+				NULL, impl->igmp_recovery.check_interval * SPA_NSEC_PER_SEC,
-		pw_log_error("can't add timer: %s", spa_strerror(res));
+				on_igmp_recovery_timer_event, impl)) < 0) {
-		goto finish;
+			pw_log_error("can't add timer: %s", spa_strerror(res));
+			goto finish;
+		}
 	}
 
 finish:

src/modules/module-rtp/audio.c

@@ -606,7 +606,7 @@ static void rtp_audio_stop_timer(struct impl *impl)
 static void rtp_audio_flush_timeout(struct impl *impl, uint64_t expirations)
 {
 	if (expirations > 1)
-		pw_log_warn("missing timeout %"PRIu64, expirations);
+		pw_log_trace("missing timeout %"PRIu64, expirations);
 	rtp_audio_flush_packets(impl, expirations, 0);
 }
 

src/modules/network-utils.h

@@ -87,6 +87,64 @@ static inline int pw_net_parse_address_port(const char *address,
 	return pw_net_parse_address(n, port, addr, len);
 }
 
+static inline bool pw_net_are_addresses_equal(const struct sockaddr_storage *addr1,
+					      const struct sockaddr_storage *addr2,
+					      bool compare_ports)
+{
+	/* IPv6 addresses might actually be mapped IPv4 ones. In cases where
+	 * such mapped IPv4 addresses are compared against plain IPv4 ones
+	 * (that is, ss_family == AF_INET), special handling is required. */
+	bool addr1_is_mapped_ipv4 = (addr1->ss_family == AF_INET6) &&
+	                            IN6_IS_ADDR_V4MAPPED(&((struct sockaddr_in6*)addr1)->sin6_addr);
+	bool addr2_is_mapped_ipv4 = (addr2->ss_family == AF_INET6) &&
+	                            IN6_IS_ADDR_V4MAPPED(&((struct sockaddr_in6*)addr2)->sin6_addr);
+
+	if ((addr1->ss_family == AF_INET) && (addr2->ss_family == AF_INET)) {
+		/* Both addresses are plain IPv4 addresses. */
+
+		const struct sockaddr_in *addr1_in = (const struct sockaddr_in *)addr1;
+		const struct sockaddr_in *addr2_in = (const struct sockaddr_in *)addr2;
+		return (!compare_ports || (addr1_in->sin_port == addr2_in->sin_port)) &&
+		       (addr1_in->sin_addr.s_addr == addr2_in->sin_addr.s_addr);
+	} else if ((addr1->ss_family == AF_INET6) && (addr2->ss_family == AF_INET6)) {
+		/* Both addresses are IPv6 addresses. (Note that this logic here
+		 * works correctly even if both are actually mapped IPv4 addresses.) */
+
+		const struct sockaddr_in6 *addr1_in6 = (const struct sockaddr_in6 *)addr1;
+		const struct sockaddr_in6 *addr2_in6 = (const struct sockaddr_in6 *)addr2;
+		return (!compare_ports || (addr1_in6->sin6_port == addr2_in6->sin6_port)) &&
+		       (addr1_in6->sin6_scope_id == addr2_in6->sin6_scope_id) &&
+		       (memcmp(&addr1_in6->sin6_addr, &addr2_in6->sin6_addr, sizeof(struct in6_addr)) == 0);
+	} else if ((addr1->ss_family == AF_INET) && addr2_is_mapped_ipv4) {
+		/* addr1 is a plain IPv4 address, addr2 is a mapped IPv4 address.
+		 * Extract the IPv4 portion of addr2 to form a plain IPv4 address
+		 * out of it, then compare the two plain IPv4 addresses. */
+
+		struct in_addr addr2_as_ipv4;
+		const struct sockaddr_in *addr1_in = (const struct sockaddr_in *)addr1;
+		const struct sockaddr_in6 *addr2_in6 = (const struct sockaddr_in6 *)addr2;
+
+		memcpy(&addr2_as_ipv4, &(addr2_in6->sin6_addr.s6_addr[12]), 4);
+
+		return (!compare_ports || (addr1_in->sin_port == addr2_in6->sin6_port)) &&
+		       (addr1_in->sin_addr.s_addr == addr2_as_ipv4.s_addr);
+	} else if (addr1_is_mapped_ipv4 && (addr2->ss_family == AF_INET)) {
+		/* addr2 is a plain IPv4 address, addr1 is a mapped IPv4 address.
+		 * Extract the IPv4 portion of addr1 to form a plain IPv4 address
+		 * out of it, then compare the two plain IPv4 addresses. */
+
+		struct in_addr addr1_as_ipv4;
+		const struct sockaddr_in6 *addr1_in6 = (const struct sockaddr_in6 *)addr1;
+		const struct sockaddr_in *addr2_in = (const struct sockaddr_in *)addr2;
+
+		memcpy(&addr1_as_ipv4, &(addr1_in6->sin6_addr.s6_addr[12]), 4);
+
+		return (!compare_ports || (addr1_in6->sin6_port == addr2_in->sin_port)) &&
+		       (addr1_as_ipv4.s_addr == addr2_in->sin_addr.s_addr);
+	} else
+		return false;
+}
+
 static inline int pw_net_get_ip(const struct sockaddr_storage *sa, char *ip, size_t len, bool *ip4, uint16_t *port)
 {
 	if (ip4)