remote: bound updates to MTU-safe UDP datagrams

CMakeLists.txt

@@ -24,7 +24,7 @@ IF(NOT HAVE_PCAP_H)
 	MESSAGE(FATAL_ERROR "pcap/pcap.h is not found")
 ENDIF()
 
-SET(SOURCES main.c local_node.c node.c sta.c policy.c ubus.c remote.c parse.c netifd.c timeout.c event.c measurement.c band_steering.c)
+SET(SOURCES main.c local_node.c node.c sta.c policy.c ubus.c remote.c remote-message.c parse.c netifd.c timeout.c event.c measurement.c band_steering.c)
 
 IF(NL_CFLAGS)
 	ADD_DEFINITIONS(${NL_CFLAGS})
@@ -44,6 +44,22 @@ TARGET_LINK_LIBRARIES(fakeap ubox ubus)
 ADD_EXECUTABLE(ap-monitor monitor.c parse.c)
 TARGET_LINK_LIBRARIES(ap-monitor ubox pcap blobmsg_json)
 
+OPTION(BUILD_TESTING "Build remote message regression tests" OFF)
+IF(BUILD_TESTING)
+  ENABLE_TESTING()
+  ADD_EXECUTABLE(test-remote-message tests/remote-message.c remote-message.c parse.c)
+  TARGET_INCLUDE_DIRECTORIES(test-remote-message PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
+  TARGET_COMPILE_OPTIONS(test-remote-message PRIVATE -UNDEBUG)
+  TARGET_LINK_LIBRARIES(test-remote-message ubox)
+  ADD_TEST(NAME remote-message COMMAND test-remote-message)
+  ADD_EXECUTABLE(test-remote-socket tests/remote-socket.c remote-message.c)
+  TARGET_INCLUDE_DIRECTORIES(test-remote-socket PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
+  TARGET_COMPILE_OPTIONS(test-remote-socket PRIVATE -UNDEBUG -ffunction-sections -fdata-sections)
+  SET_TARGET_PROPERTIES(test-remote-socket PROPERTIES LINK_FLAGS "-Wl,--gc-sections")
+  TARGET_LINK_LIBRARIES(test-remote-socket ubox)
+  ADD_TEST(NAME remote-socket COMMAND test-remote-socket)
+ENDIF()
+
 SET(CMAKE_INSTALL_PREFIX /usr)
 
 INSTALL(TARGETS usteerd

remote-message.c

@@ -0,0 +1,142 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "remote-message.h"
+#include "remote.h"
+
+size_t usteer_message_payload_limit(unsigned int mtu, bool ipv6)
+{
+	unsigned int overhead = ipv6 ? 48 : 28;
+
+	if (mtu <= overhead)
+		return 0;
+
+	return mtu - overhead < USTEER_REMOTE_MAX_PAYLOAD ?
+		mtu - overhead : USTEER_REMOTE_MAX_PAYLOAD;
+}
+
+/* The input is a locally constructed message, not an untrusted receive buffer. */
+static struct blob_attr *message_field(struct blob_attr *data, unsigned int id)
+{
+	struct blob_attr *cur;
+	int rem;
+
+	blob_for_each_attr(cur, data, rem)
+		if (blob_id(cur) == id)
+			return cur;
+
+	return NULL;
+}
+
+static size_t metadata_size(struct blob_attr *data, unsigned int skip)
+{
+	struct blob_attr *cur;
+	size_t len = sizeof(*data);
+	int rem;
+
+	blob_for_each_attr(cur, data, rem)
+		if (blob_id(cur) != skip)
+			len += blob_pad_len(cur);
+
+	return len;
+}
+
+static size_t copy_metadata(char *out, struct blob_attr *data, unsigned int skip)
+{
+	struct blob_attr *cur;
+	size_t len = sizeof(*data);
+	int rem;
+
+	memcpy(out, data, sizeof(*data));
+	blob_for_each_attr(cur, data, rem) {
+		if (blob_id(cur) == skip)
+			continue;
+		memcpy(out + len, cur, blob_pad_len(cur));
+		len += blob_pad_len(cur);
+	}
+
+	return len;
+}
+
+static int emit_chunk(char *out, size_t len, size_t nodes, size_t node,
+		      size_t stations, usteer_message_emit emit, void *priv)
+{
+	blob_set_raw_len((struct blob_attr *)out, len);
+	blob_set_raw_len((struct blob_attr *)(out + nodes), len - nodes);
+	blob_set_raw_len((struct blob_attr *)(out + node), len - node);
+	blob_set_raw_len((struct blob_attr *)(out + stations), len - stations);
+	return emit((struct blob_attr *)out, priv);
+}
+
+int usteer_message_send(struct blob_attr *data, size_t limit,
+			usteer_message_emit emit, void *priv)
+{
+	struct blob_attr *nodes, *node, *stations, *sta;
+	size_t top_len, node_len, overhead, nodes_off, node_off, sta_off, len;
+	int rem, sta_rem, ret = 0;
+	char *out;
+
+	if (blob_pad_len(data) <= limit)
+		return emit(data, priv);
+
+	nodes = message_field(data, APMSG_NODES);
+	if (!nodes)
+		return -EINVAL;
+
+	top_len = metadata_size(data, APMSG_NODES) + sizeof(*nodes);
+	if (top_len > limit)
+		return -EMSGSIZE;
+
+	/* Validate the entire update before emitting any part of it. Metadata and
+	 * individual station records are indivisible in the existing protocol.
+	 * Never silently truncate them or fall back to IP fragmentation.
+	 */
+	blob_for_each_attr(node, nodes, rem) {
+		stations = message_field(node, APMSG_NODE_STATIONS);
+		if (!stations)
+			return -EINVAL;
+
+		node_len = metadata_size(node, APMSG_NODE_STATIONS) + sizeof(*stations);
+		if (node_len > limit - top_len)
+			return -EMSGSIZE;
+
+		overhead = top_len + node_len;
+		blob_for_each_attr(sta, stations, sta_rem)
+			if (blob_pad_len(sta) > limit - overhead)
+				return -EMSGSIZE;
+	}
+
+	out = malloc(limit);
+	if (!out)
+		return -ENOMEM;
+
+	nodes_off = copy_metadata(out, data, APMSG_NODES);
+	memcpy(out + nodes_off, nodes, sizeof(*nodes));
+	node_off = nodes_off + sizeof(*nodes);
+	blob_for_each_attr(node, nodes, rem) {
+		stations = message_field(node, APMSG_NODE_STATIONS);
+		sta_off = node_off + copy_metadata(out + node_off, node, APMSG_NODE_STATIONS);
+		memcpy(out + sta_off, stations, sizeof(*stations));
+		overhead = sta_off + sizeof(*stations);
+		len = overhead;
+
+		blob_for_each_attr(sta, stations, sta_rem) {
+			if (blob_pad_len(sta) > limit - len) {
+				ret = emit_chunk(out, len, nodes_off, node_off, sta_off, emit, priv);
+				if (ret)
+					goto out;
+				len = overhead;
+			}
+			memcpy(out + len, sta, blob_pad_len(sta));
+			len += blob_pad_len(sta);
+		}
+		ret = emit_chunk(out, len, nodes_off, node_off, sta_off, emit, priv);
+		if (ret)
+			goto out;
+	}
+out:
+	free(out);
+	return ret;
+}

remote-message.h

@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __USTEER_REMOTE_MESSAGE_H
+#define __USTEER_REMOTE_MESSAGE_H
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <libubox/blob.h>
+
+/* Leave room for IPv6 and UDP even on a 1280-byte link. */
+#define USTEER_REMOTE_MAX_PAYLOAD 1200
+
+typedef int (*usteer_message_emit)(struct blob_attr *data, void *priv);
+
+size_t usteer_message_payload_limit(unsigned int mtu, bool ipv6);
+int usteer_message_send(struct blob_attr *data, size_t limit,
+			usteer_message_emit emit, void *priv);
+
+#endif

remote.c

@@ -21,6 +21,7 @@
 
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/ioctl.h>
 #include <netinet/in.h>
 #include <net/if.h>
 #include <arpa/inet.h>
@@ -32,10 +33,11 @@
 #include <libubox/usock.h>
 #include "usteer.h"
 #include "remote.h"
+#include "remote-message.h"
 #include "node.h"
 
 static uint32_t local_id;
-static struct uloop_fd remote_fd;
+static struct uloop_fd remote_fd = { .fd = -1 };
 static struct uloop_timeout remote_timer;
 static struct uloop_timeout reload_timer;
 
@@ -485,7 +487,7 @@ static void interface_recv_v6(struct uloop_fd *u, unsigned int events){
 	} while (1);
 }
 
-static void interface_send_msg_v4(struct interface *iface, struct blob_attr *data)
+static int interface_send_msg_v4(struct interface *iface, struct blob_attr *data)
 {
 	static size_t cmsg_data[( CMSG_SPACE(sizeof(struct in_pktinfo)) / sizeof(size_t)) + 1];
 	static struct sockaddr_in a;
@@ -518,11 +520,12 @@ static void interface_send_msg_v4(struct interface *iface, struct blob_attr *dat
 	iov.iov_len = blob_pad_len(data);
 
 	if (sendmsg(remote_fd.fd, &m, 0) < 0)
-		perror("sendmsg");
+		return -errno;
+	return 0;
 }
 
 
-static void interface_send_msg_v6(struct interface *iface, struct blob_attr *data) {
+static int interface_send_msg_v6(struct interface *iface, struct blob_attr *data) {
 	static struct sockaddr_in6 groupSock = {};
 
 	groupSock.sin6_family = AF_INET6;
@@ -532,15 +535,42 @@ static void interface_send_msg_v6(struct interface *iface, struct blob_attr *dat
 	setsockopt(remote_fd.fd, IPPROTO_IPV6, IPV6_MULTICAST_IF, &iface->ifindex, sizeof(iface->ifindex));
 
 	if (sendto(remote_fd.fd, data, blob_pad_len(data), 0, (const struct sockaddr *)&groupSock, sizeof(groupSock)) < 0)
-		perror("sendmsg");
+		return -errno;
+	return 0;
 }
 
-static void interface_send_msg(struct interface *iface, struct blob_attr *data){
+static int interface_emit_msg(struct blob_attr *data, void *priv)
+{
+	struct interface *iface = priv;
+
 	if (config.ipv6) {
-		interface_send_msg_v6(iface, data);
+		return interface_send_msg_v6(iface, data);
 	} else {
-		interface_send_msg_v4(iface, data);
+		return interface_send_msg_v4(iface, data);
+	}
+}
+
+static void interface_send_msg(struct interface *iface, struct blob_attr *data)
+{
+	struct ifreq ifr = {};
+	size_t limit;
+	int ret;
+
+	/* Node notifications can arrive before the remote socket is ready. */
+	if (!remote_fd.registered)
+		return;
+
+	snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", interface_name(iface));
+	if (ioctl(remote_fd.fd, SIOCGIFMTU, &ifr) < 0) {
+		MSG(FATAL, "Cannot read MTU for %s: %s\n", interface_name(iface), strerror(errno));
+		return;
 	}
+	limit = usteer_message_payload_limit(ifr.ifr_mtu > 0 ? ifr.ifr_mtu : 0,
+					    config.ipv6);
+	ret = usteer_message_send(data, limit, interface_emit_msg, iface);
+	if (ret)
+		MSG(FATAL, "Cannot send remote update on %s (payload limit %zu): %s\n",
+		    interface_name(iface), limit, strerror(-ret));
 }
 
 static void usteer_send_sta_info(struct sta_info *sta)
@@ -744,7 +774,7 @@ static int usteer_create_v6_socket() {
 
 static void usteer_reload_timer(struct uloop_timeout *t) {
 	/* Remove uloop descriptor */
-	if (remote_fd.fd && remote_fd.registered) {
+	if (remote_fd.fd >= 0) {
 		uloop_fd_delete(&remote_fd);
 		close(remote_fd.fd);
 	}
@@ -768,11 +798,11 @@ int usteer_interface_init(void)
 	if (usteer_init_local_id())
 		return -1;
 
-	remote_timer.cb = usteer_send_update_timer;
-	remote_timer.cb(&remote_timer);
-
 	reload_timer.cb = usteer_reload_timer;
 	reload_timer.cb(&reload_timer);
 
+	remote_timer.cb = usteer_send_update_timer;
+	remote_timer.cb(&remote_timer);
+
 	return 0;
 }

tests/README.md

@@ -0,0 +1,54 @@
+# Testing bounded remote updates
+
+With the regular Usteer build dependencies installed:
+
+```sh
+cmake -S . -B build-test -DBUILD_TESTING=ON
+cmake --build build-test
+ctest --test-dir build-test --output-on-failure
+```
+
+For ASan/UBSan, also pass these options when configuring:
+
+```sh
+-DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer" \
+-DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined"
+```
+
+The test uses the unchanged parser from `parse.c`. It checks byte-identical
+forwarding of small messages, limits exactly at and just below the message
+size, and 1,001 payload limits from 200 to 1,200 bytes with 300 stations
+across two nodes plus a third, empty node. Station records must not be lost
+or duplicated. Metadata, unknown node fields and the input buffer must be
+preserved. Further cases cover indivisible oversized metadata in the last
+node, oversized station records, small MTUs, host-only updates and send
+errors. Two tests send and receive messages through actual IPv4/IPv6
+loopback UDP sockets; IPv6 loopback must be available in the test environment.
+
+`remote-socket` also exercises the actual send path with mocked system
+calls: no MTU query before socket registration or after removal, valid
+descriptor 0 and a controlled MTU query failure. The initial periodic send
+is started only after socket initialization.
+
+## Protocol and limitations
+
+Splitting uses only existing message fields. Each chunk contains the same
+host metadata and sequence number as the logical update. The existing
+receiver does not use the sequence number to suppress duplicates and
+updates station records individually; missing stations are not deleted
+because an update contains only a subset. No new receive logic or
+simultaneous upgrade of all peers is required. Parser and data tests
+support this, but do not replace a multi-AP deployment test.
+
+The UDP payload limit is at most 1,200 bytes and is reduced further for
+smaller local interface MTUs, accounting for IPv4/IPv6 and UDP headers.
+This is not general path-MTU discovery for tunnels or routed networks.
+The entire message is checked for splittability before sending any chunk.
+An indivisible oversized metadata block or station record produces
+`EMSGSIZE` and a logged send error, not truncation or an oversized fallback.
+A later socket send error can still leave a partially received update,
+as with ordinary UDP loss; subsequent periodic updates continue normally.
+
+The patch is intended to avoid IP fragmentation of Usteer updates on the
+intended LAN interfaces. It does not establish a fix for unrelated radio
+or client connectivity problems.