Netdev List
 help / color / mirror / Atom feed
* [PATCH net 0/2] net: fix SO_RESERVE_MEM socket type restriction and value bounds
@ 2026-09-24 12:36 Eric Dumazet
  2026-09-24 12:36 ` [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value Eric Dumazet
  2026-09-24 12:36 ` [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test Eric Dumazet
  0 siblings, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-09-24 12:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Wei Wang, Simon Horman, Cai Xinchen, netdev, edumazet,
	Eric Dumazet

This series fixes two issues with SO_RESERVE_MEM and adds a kselftest suite:

- Patch 1 restricts sock_reserve_memory() to TCP sockets (sk_is_tcp(sk))
  instead of sk_has_account(sk), and caps the requested reservation to
  INT_MAX >> 1 in sk_setsockopt(SO_RESERVE_MEM).

  Currently, sk_has_account(sk) allows SO_RESERVE_MEM on UDP sockets,
  where sk->sk_forward_alloc is protected by sk->sk_receive_queue.lock
  rather than the socket lock, and udp_rmem_release() does not account
  for sk_unused_reserved_mem(sk). Concurrent UDP packet processing and
  setsockopt(SO_RESERVE_MEM) corrupt sk_forward_alloc and memcg
  accounting. In addition, values near INT_MAX overflow 32-bit signed
  int in sk_mem_pages(delta) and (pages << PAGE_SHIFT).

- Patch 2 adds a kselftest (tools/testing/selftests/net/so_reserve_mem.c)
  covering non-TCP rejection (UDP, AF_UNIX, SOCK_RAW, MPTCP), bounds
  validation, reserve grow/shrink/rounding and SK_MEMINFO_FWD_ALLOC
  reporting, cgroup v2 memory.max enforcement, accept() child zero-reserve
  inheritance, and reserve retention across TCP traffic.

Eric Dumazet (2):
  net: restrict SO_RESERVE_MEM to TCP sockets and cap max value
  selftests/net: add SO_RESERVE_MEM test

 net/core/sock.c                              |   4 +-
 tools/testing/selftests/net/.gitignore       |   1 +
 tools/testing/selftests/net/Makefile         |   1 +
 tools/testing/selftests/net/so_reserve_mem.c | 333 +++++++++++++++++++
 4 files changed, 337 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/so_reserve_mem.c

-- 
2.56.0.rc1.310.g51773c2048-goog


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value
  2026-09-24 12:36 [PATCH net 0/2] net: fix SO_RESERVE_MEM socket type restriction and value bounds Eric Dumazet
@ 2026-09-24 12:36 ` Eric Dumazet
  2026-09-24 17:32   ` Wei Wang
  2026-09-25 12:49   ` netdev-bot+sashiko
  2026-09-24 12:36 ` [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test Eric Dumazet
  1 sibling, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-09-24 12:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Wei Wang, Simon Horman, Cai Xinchen, netdev, edumazet,
	Eric Dumazet

Commit 2bb2f5fb21b0 ("net: add new socket option SO_RESERVE_MEM") and
commit d00c8ee31729 ("net: fix possible NULL deref in sock_reserve_memory")
only checked sk_has_account(sk), which is true for both TCP and UDP
sockets.

However, SO_RESERVE_MEM and sk_unused_reserved_mem() are currently only
supported by TCP:
- On UDP sockets, sk->sk_forward_alloc is protected by
  sk->sk_receive_queue.lock, whereas sock_reserve_memory() and
  sock_release_reserved_memory() only acquire lock_sock(sk). Concurrent
  UDP packet reception/release and setsockopt(SO_RESERVE_MEM) corrupt
  sk_forward_alloc and memcg accounting.
- udp_rmem_release() reclaims excess sk_forward_alloc without
  accounting for sk_unused_reserved_mem(sk).

Restrict sock_reserve_memory() to TCP sockets (sk_is_tcp(sk)) for now.
Supporting SO_RESERVE_MEM for UDP (acquiring sk_receive_queue.lock and
honoring sk_unused_reserved_mem() in udp_rmem_release()) can be done in
a future net-next series if needed.

In addition, reject val > INT_MAX >> 1 with -EINVAL in
sk_setsockopt(SO_RESERVE_MEM). Without an upper bound, values near
INT_MAX cause sk_mem_pages(delta) and (pages << PAGE_SHIFT) to overflow
32-bit signed int, corrupting sk->sk_forward_alloc and
sk->sk_reserved_mem.

Fixes: 2bb2f5fb21b0 ("net: add new socket option SO_RESERVE_MEM")
Reported-by: Cai Xinchen <caixinchen1@huawei.com>
Closes: https://lore.kernel.org/netdev/5a88421d-10ef-4fca-9acb-85a27a3c1173@huawei.com/
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/sock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index d23333bb4f3fafa19f58095522ef10d918d92496..cfc6700a62e6fb904f4c8320509d7360c6223b1f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1034,7 +1034,7 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
 	bool charged;
 	int pages;
 
-	if (!mem_cgroup_sk_enabled(sk) || !sk_has_account(sk))
+	if (!mem_cgroup_sk_enabled(sk) || !sk_is_tcp(sk))
 		return -EOPNOTSUPP;
 
 	if (!bytes)
@@ -1661,7 +1661,7 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
 	{
 		int delta;
 
-		if (val < 0) {
+		if (val < 0 || val > INT_MAX >> 1) {
 			ret = -EINVAL;
 			break;
 		}
-- 
2.56.0.rc1.310.g51773c2048-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test
  2026-09-24 12:36 [PATCH net 0/2] net: fix SO_RESERVE_MEM socket type restriction and value bounds Eric Dumazet
  2026-09-24 12:36 ` [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value Eric Dumazet
@ 2026-09-24 12:36 ` Eric Dumazet
  2026-09-25 12:49   ` netdev-bot+sashiko
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2026-09-24 12:36 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Wei Wang, Simon Horman, Cai Xinchen, netdev, edumazet,
	Eric Dumazet

Add a kselftest suite covering SO_RESERVE_MEM behavior:
- Reject unsupported socket types (UDP/SOCK_DGRAM, UNIX/SOCK_STREAM,
  SOCK_RAW, MPTCP) with EOPNOTSUPP.
- Validate bounds (negative values and values > INT_MAX >> 1 return
  EINVAL).
- Verify reserve adjustments (increase, decrease, page rounding, and
  reset to 0) and corresponding sk_meminfo[SK_MEMINFO_FWD_ALLOC] changes
  via getsockopt(SO_MEMINFO).
- Exercise cgroup v2 memory.max enforcement returning ENOMEM when the
  requested reservation exceeds the cgroup limit.
- Verify that a child socket returned by accept() starts with a 0
  reserve even when its parent listener configured a non-zero
  SO_RESERVE_MEM.
- Verify reserve retention across TCP send/receive/ACK cycles.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Assisted-by: LLM
---
 tools/testing/selftests/net/.gitignore       |   1 +
 tools/testing/selftests/net/Makefile         |   1 +
 tools/testing/selftests/net/so_reserve_mem.c | 333 +++++++++++++++++++
 3 files changed, 335 insertions(+)
 create mode 100644 tools/testing/selftests/net/so_reserve_mem.c

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index c9f46031ac73b20850d068125bb804420d236ad8..a15d7cc4fdcb22b36fdb35b9eaf47e511caa8673 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -41,6 +41,7 @@ skf_net_off
 socket
 so_incoming_cpu
 so_netns_cookie
+so_reserve_mem
 so_rcv_listener
 stress_reuseport_listen
 tap
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 3ee3378f8b26eff9d99a834491ad5ef594dcd782..300207a840c1bcb73a41151c8f94e8f78d01a29b 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -196,6 +196,7 @@ TEST_GEN_PROGS := \
 	sk_connect_zero_addr \
 	sk_so_peek_off \
 	so_incoming_cpu \
+	so_reserve_mem \
 	tap \
 	tcp_port_share \
 	tls \
diff --git a/tools/testing/selftests/net/so_reserve_mem.c b/tools/testing/selftests/net/so_reserve_mem.c
new file mode 100644
index 0000000000000000000000000000000000000000..c2ca02ebe8353b858a94b7f1e026bad10bf9aeeb
--- /dev/null
+++ b/tools/testing/selftests/net/so_reserve_mem.c
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <linux/sock_diag.h>
+#include <netinet/in.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "kselftest_harness.h"
+
+#ifndef SO_RESERVE_MEM
+#define SO_RESERVE_MEM 73
+#endif
+
+#ifndef IPPROTO_MPTCP
+#define IPPROTO_MPTCP 262
+#endif
+
+static int get_reserve_mem(struct __test_metadata *_metadata, int fd)
+{
+	int val = -1;
+	socklen_t len = sizeof(val);
+
+	EXPECT_EQ(getsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, &len), 0);
+	return val;
+}
+
+static __u32 get_fwd_alloc(struct __test_metadata *_metadata, int fd)
+{
+	__u32 meminfo[SK_MEMINFO_VARS] = {};
+	socklen_t len = sizeof(meminfo);
+
+	EXPECT_EQ(getsockopt(fd, SOL_SOCKET, SO_MEMINFO, meminfo, &len), 0);
+	return meminfo[SK_MEMINFO_FWD_ALLOC];
+}
+
+FIXTURE(so_reserve_mem)
+{
+	char cg_root[64];
+	char cg_child[128];
+	long page_size;
+	bool cg_mounted;
+};
+
+FIXTURE_SETUP(so_reserve_mem)
+{
+	char procs_path[160], ctrl_path[160];
+	int fd, ret;
+
+	self->page_size = sysconf(_SC_PAGESIZE);
+	ASSERT_GT(self->page_size, 0);
+
+	if (unshare(CLONE_NEWNS))
+		SKIP(return, "Failed to unshare mount namespace (need root)");
+
+	mount("none", "/", NULL, MS_REC | MS_PRIVATE, NULL);
+
+	snprintf(self->cg_root, sizeof(self->cg_root),
+		 "/tmp/ksft_so_reserve_XXXXXX");
+	ASSERT_NE(mkdtemp(self->cg_root), NULL);
+
+	if (mount("none", self->cg_root, "cgroup2", 0, NULL)) {
+		rmdir(self->cg_root);
+		SKIP(return, "Failed to mount cgroup2 (need root)");
+	}
+
+	snprintf(ctrl_path, sizeof(ctrl_path), "%s/cgroup.subtree_control",
+		 self->cg_root);
+	fd = open(ctrl_path, O_WRONLY);
+	if (fd >= 0) {
+		ret = write(fd, "+memory", 7);
+		(void)ret;
+		close(fd);
+	}
+
+	snprintf(self->cg_child, sizeof(self->cg_child), "%s/test_%d",
+		 self->cg_root, getpid());
+	if (mkdir(self->cg_child, 0755)) {
+		umount2(self->cg_root, MNT_DETACH);
+		rmdir(self->cg_root);
+		ASSERT_TRUE(false);
+	}
+
+	snprintf(procs_path, sizeof(procs_path), "%s/cgroup.procs",
+		 self->cg_child);
+	fd = open(procs_path, O_WRONLY);
+	if (fd < 0 || dprintf(fd, "%d\n", getpid()) <= 0) {
+		if (fd >= 0)
+			close(fd);
+		rmdir(self->cg_child);
+		umount2(self->cg_root, MNT_DETACH);
+		rmdir(self->cg_root);
+		ASSERT_TRUE(false);
+	}
+	close(fd);
+	self->cg_mounted = true;
+}
+
+FIXTURE_TEARDOWN(so_reserve_mem)
+{
+	char procs_path[160];
+	int fd;
+
+	if (!self->cg_mounted)
+		return;
+
+	snprintf(procs_path, sizeof(procs_path), "%s/cgroup.procs",
+		 self->cg_root);
+	fd = open(procs_path, O_WRONLY);
+	if (fd >= 0) {
+		dprintf(fd, "%d\n", getpid());
+		close(fd);
+	}
+	rmdir(self->cg_child);
+	umount2(self->cg_root, MNT_DETACH);
+	rmdir(self->cg_root);
+}
+
+static void check_non_tcp_rejected(struct __test_metadata *_metadata,
+				   int domain, int type, int protocol,
+				   int val)
+{
+	int fd = socket(domain, type, protocol);
+
+	if (fd < 0) {
+		EXPECT_TRUE(errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT);
+		return;
+	}
+	EXPECT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), -1);
+	EXPECT_EQ(errno, EOPNOTSUPP);
+	close(fd);
+}
+
+TEST_F(so_reserve_mem, non_tcp_rejected)
+{
+	int val = self->page_size * 4;
+
+	check_non_tcp_rejected(_metadata, AF_INET, SOCK_DGRAM, 0, val);
+	check_non_tcp_rejected(_metadata, AF_UNIX, SOCK_STREAM, 0, val);
+	check_non_tcp_rejected(_metadata, AF_INET, SOCK_RAW, IPPROTO_ICMP, val);
+	check_non_tcp_rejected(_metadata, AF_INET, SOCK_STREAM, IPPROTO_MPTCP, val);
+}
+
+TEST_F(so_reserve_mem, grow_shrink_and_rounding)
+{
+	int ps = self->page_size;
+	int fd, val;
+
+	fd = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(fd, 0);
+
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), 0);
+	EXPECT_EQ(get_fwd_alloc(_metadata, fd), 0U);
+
+	/* Negative or > INT_MAX >> 1 value -> EINVAL */
+	val = -1;
+	EXPECT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	val = (INT_MAX >> 1) + 1;
+	EXPECT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	val = INT_MAX;
+	EXPECT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	/* 1 byte rounds up to 1 page */
+	val = 1;
+	ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), ps);
+	EXPECT_EQ(get_fwd_alloc(_metadata, fd), (__u32)ps);
+
+	/* Grow to 16 pages */
+	val = 16 * ps;
+	ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), 16 * ps);
+	EXPECT_EQ(get_fwd_alloc(_metadata, fd), (__u32)(16 * ps));
+
+	/* Shrink by 1 byte (rounds delta down to 0 -> stays 16 pages) */
+	val = 16 * ps - 1;
+	ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), 16 * ps);
+	EXPECT_EQ(get_fwd_alloc(_metadata, fd), (__u32)(16 * ps));
+
+	/* Shrink to 4 pages */
+	val = 4 * ps;
+	ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), 4 * ps);
+	EXPECT_EQ(get_fwd_alloc(_metadata, fd), (__u32)(4 * ps));
+
+	/* Release all */
+	val = 0;
+	ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), 0);
+	EXPECT_EQ(get_fwd_alloc(_metadata, fd), 0U);
+
+	close(fd);
+}
+
+TEST_F(so_reserve_mem, cgroup_memory_max)
+{
+	char max_path[160];
+	int ps = self->page_size;
+	int fd, max_fd, val;
+
+	snprintf(max_path, sizeof(max_path), "%s/memory.max", self->cg_child);
+	max_fd = open(max_path, O_WRONLY);
+	if (max_fd < 0)
+		SKIP(return, "cgroup memory controller not delegated");
+
+	fd = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(fd, 0);
+
+	/* Limit cgroup memory to 8 pages and try to reserve 64 pages */
+	ASSERT_GT(dprintf(max_fd, "%ld\n", 8L * ps), 0);
+
+	val = 64 * ps;
+	EXPECT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), -1);
+	EXPECT_EQ(errno, ENOMEM);
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), 0);
+
+	/* Restore unlimited memory.max */
+	ASSERT_GT(dprintf(max_fd, "max\n"), 0);
+	close(max_fd);
+
+	val = 4 * ps;
+	ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, fd), 4 * ps);
+
+	close(fd);
+}
+
+TEST_F(so_reserve_mem, accept_child_zero_reserve)
+{
+	struct sockaddr_in addr = {
+		.sin_family = AF_INET,
+		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
+	};
+	socklen_t alen = sizeof(addr);
+	int ps = self->page_size;
+	int lfd, cfd, sfd, val;
+
+	lfd = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(lfd, 0);
+
+	/* Set SO_RESERVE_MEM on listener before listen() */
+	val = 4 * ps;
+	ASSERT_EQ(setsockopt(lfd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	ASSERT_EQ(bind(lfd, (struct sockaddr *)&addr, sizeof(addr)), 0);
+	ASSERT_EQ(listen(lfd, 2), 0);
+	ASSERT_EQ(getsockname(lfd, (struct sockaddr *)&addr, &alen), 0);
+
+	/* Grow SO_RESERVE_MEM on listener after listen() */
+	val = 8 * ps;
+	ASSERT_EQ(setsockopt(lfd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, lfd), 8 * ps);
+	EXPECT_EQ(get_fwd_alloc(_metadata, lfd), (__u32)(8 * ps));
+
+	cfd = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(cfd, 0);
+	ASSERT_EQ(connect(cfd, (struct sockaddr *)&addr, sizeof(addr)), 0);
+
+	sfd = accept(lfd, NULL, NULL);
+	ASSERT_GE(sfd, 0);
+
+	/* Child after accept() must have 0 reserve while listener keeps 8 pages */
+	EXPECT_EQ(get_reserve_mem(_metadata, sfd), 0);
+	EXPECT_EQ(get_fwd_alloc(_metadata, sfd), 0U);
+	EXPECT_EQ(get_reserve_mem(_metadata, lfd), 8 * ps);
+	EXPECT_EQ(get_fwd_alloc(_metadata, lfd), (__u32)(8 * ps));
+
+	/* Child can still independently set its own SO_RESERVE_MEM */
+	val = 6 * ps;
+	ASSERT_EQ(setsockopt(sfd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	EXPECT_EQ(get_reserve_mem(_metadata, sfd), 6 * ps);
+	EXPECT_EQ(get_fwd_alloc(_metadata, sfd), (__u32)(6 * ps));
+
+	close(sfd);
+	close(cfd);
+	close(lfd);
+}
+
+TEST_F(so_reserve_mem, preserved_after_traffic)
+{
+	struct sockaddr_in addr = {
+		.sin_family = AF_INET,
+		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
+	};
+	socklen_t alen = sizeof(addr);
+	int ps = self->page_size;
+	int lfd, cfd, sfd, val;
+	char buf[8192] = {};
+
+	lfd = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(lfd, 0);
+	ASSERT_EQ(bind(lfd, (struct sockaddr *)&addr, sizeof(addr)), 0);
+	ASSERT_EQ(listen(lfd, 1), 0);
+	ASSERT_EQ(getsockname(lfd, (struct sockaddr *)&addr, &alen), 0);
+
+	cfd = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_GE(cfd, 0);
+	val = 16 * ps;
+	ASSERT_EQ(setsockopt(cfd, SOL_SOCKET, SO_RESERVE_MEM, &val, sizeof(val)), 0);
+	ASSERT_EQ(connect(cfd, (struct sockaddr *)&addr, sizeof(addr)), 0);
+
+	sfd = accept(lfd, NULL, NULL);
+	ASSERT_GE(sfd, 0);
+
+	/* Send & drain traffic; cfd must retain its 16-page forward alloc,
+	 * while sfd (0 reserve) reclaims its forward alloc back to 0.
+	 */
+	ASSERT_EQ(send(cfd, buf, sizeof(buf), 0), (ssize_t)sizeof(buf));
+	ASSERT_EQ(recv(sfd, buf, sizeof(buf), MSG_WAITALL), (ssize_t)sizeof(buf));
+
+	EXPECT_GE(get_fwd_alloc(_metadata, cfd), (__u32)(16 * ps));
+	EXPECT_EQ(get_fwd_alloc(_metadata, sfd), 0U);
+
+	close(sfd);
+	close(cfd);
+	close(lfd);
+}
+
+TEST_HARNESS_MAIN
-- 
2.56.0.rc1.310.g51773c2048-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value
  2026-09-24 12:36 ` [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value Eric Dumazet
@ 2026-09-24 17:32   ` Wei Wang
  2026-09-25 12:49   ` netdev-bot+sashiko
  1 sibling, 0 replies; 9+ messages in thread
From: Wei Wang @ 2026-09-24 17:32 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Cai Xinchen, netdev, edumazet

On Thu, Sep 24, 2026 at 5:36 AM Eric Dumazet <edumazet@google.com> wrote:
>
> Commit 2bb2f5fb21b0 ("net: add new socket option SO_RESERVE_MEM") and
> commit d00c8ee31729 ("net: fix possible NULL deref in sock_reserve_memory")
> only checked sk_has_account(sk), which is true for both TCP and UDP
> sockets.
>
> However, SO_RESERVE_MEM and sk_unused_reserved_mem() are currently only
> supported by TCP:
> - On UDP sockets, sk->sk_forward_alloc is protected by
>   sk->sk_receive_queue.lock, whereas sock_reserve_memory() and
>   sock_release_reserved_memory() only acquire lock_sock(sk). Concurrent
>   UDP packet reception/release and setsockopt(SO_RESERVE_MEM) corrupt
>   sk_forward_alloc and memcg accounting.
> - udp_rmem_release() reclaims excess sk_forward_alloc without
>   accounting for sk_unused_reserved_mem(sk).
>
> Restrict sock_reserve_memory() to TCP sockets (sk_is_tcp(sk)) for now.
> Supporting SO_RESERVE_MEM for UDP (acquiring sk_receive_queue.lock and
> honoring sk_unused_reserved_mem() in udp_rmem_release()) can be done in
> a future net-next series if needed.
>
> In addition, reject val > INT_MAX >> 1 with -EINVAL in
> sk_setsockopt(SO_RESERVE_MEM). Without an upper bound, values near
> INT_MAX cause sk_mem_pages(delta) and (pages << PAGE_SHIFT) to overflow
> 32-bit signed int, corrupting sk->sk_forward_alloc and
> sk->sk_reserved_mem.
>
> Fixes: 2bb2f5fb21b0 ("net: add new socket option SO_RESERVE_MEM")
> Reported-by: Cai Xinchen <caixinchen1@huawei.com>
> Closes: https://lore.kernel.org/netdev/5a88421d-10ef-4fca-9acb-85a27a3c1173@huawei.com/
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/core/sock.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Wei Wang <weiwan@google.com>

> diff --git a/net/core/sock.c b/net/core/sock.c
> index d23333bb4f3fafa19f58095522ef10d918d92496..cfc6700a62e6fb904f4c8320509d7360c6223b1f 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1034,7 +1034,7 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
>         bool charged;
>         int pages;
>
> -       if (!mem_cgroup_sk_enabled(sk) || !sk_has_account(sk))
> +       if (!mem_cgroup_sk_enabled(sk) || !sk_is_tcp(sk))
>                 return -EOPNOTSUPP;
>
>         if (!bytes)
> @@ -1661,7 +1661,7 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
>         {
>                 int delta;
>
> -               if (val < 0) {
> +               if (val < 0 || val > INT_MAX >> 1) {
>                         ret = -EINVAL;
>                         break;
>                 }
> --
> 2.56.0.rc1.310.g51773c2048-goog
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value
  2026-09-24 12:36 ` [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value Eric Dumazet
  2026-09-24 17:32   ` Wei Wang
@ 2026-09-25 12:49   ` netdev-bot+sashiko
  1 sibling, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-25 12:49 UTC (permalink / raw)
  To: edumazet
  Cc: davem, kuba, pabeni, weiwan, horms, caixinchen1, netdev, edumazet

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] getsockopt and setsockopt for SO_RESERVE_MEM now disagree on the
  valid range.

--

net: restrict SO_RESERVE_MEM to TCP sockets and cap max value

This patch limits sock_reserve_memory() to TCP sockets. UDP protects
sk_forward_alloc with a different lock, and udp_rmem_release() does not
honor the reserved memory. It also makes sk_setsockopt(SO_RESERVE_MEM)
reject values above INT_MAX >> 1, so that the page conversions in
sock_reserve_memory() cannot overflow.

Closes: https://lore.kernel.org/netdev/5a88421d-10ef-4fca-9acb-85a27a3c1173@huawei.com/

> diff --git a/net/core/sock.c b/net/core/sock.c
> index d23333bb4f3fa..cfc6700a62e6f 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c

[ ... ]

> @@ -1661,7 +1661,7 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
>  	{
>  		int delta;
>  
> -		if (val < 0) {
> +		if (val < 0 || val > INT_MAX >> 1) {

[Severity: Low]
Could this cap leave getsockopt and setsockopt disagreeing about the
valid range for SO_RESERVE_MEM?

The check is made on the raw byte count. sock_reserve_memory() then rounds
the request up to whole pages before it stores the value:

net/core/sock.c:sock_reserve_memory() {
    ...
	pages = sk_mem_pages(bytes);
    ...
	WRITE_ONCE(sk->sk_reserved_mem,
		   sk->sk_reserved_mem + (pages << PAGE_SHIFT));
    ...
}

If val is 0x3FFFFFFF, the largest value this check accepts, sk_mem_pages()
returns 0x40000 pages with 4K pages (0x4000 with 64K pages). So
sk_reserved_mem becomes 0x40000000. The same holds for any val within one
page below the cap.

sk_getsockopt() then reports that rounded value back to userspace:

	case SO_RESERVE_MEM:
		v.val = READ_ONCE(sk->sk_reserved_mem);

If that value (1073741824) is passed back to setsockopt(SO_RESERVE_MEM), as
a save/restore or checkpoint/restore flow would, it is now above
INT_MAX >> 1 and fails with -EINVAL. Before this patch, setting the value
returned by getsockopt always worked.

Nothing overflows here, since 0x40000000 still fits in an int. The only
effect is an unexpected -EINVAL on that round trip.

Would it make sense to apply the cap after page rounding? For example,
reject when PAGE_ALIGN(val) > INT_MAX >> 1, or state the cap in
page-aligned terms.

>  			ret = -EINVAL;
>  			break;
>  		}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924123602.1979090-1-edumazet%40google.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test
  2026-09-24 12:36 ` [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test Eric Dumazet
@ 2026-09-25 12:49   ` netdev-bot+sashiko
  2026-09-25 13:06     ` Eric Dumazet
  2026-09-25 13:49     ` Matthieu Baerts
  0 siblings, 2 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-25 12:49 UTC (permalink / raw)
  To: edumazet
  Cc: davem, kuba, pabeni, weiwan, horms, caixinchen1, netdev, edumazet

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 4 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new SO_RESERVE_MEM selftest needs memcg socket accounting
  but never checks for it, so on systems without it the tests fail…
- [Medium] When socket() fails, check_non_tcp_rejected() only accepts
  EAFNOSUPPORT or EPROTONOSUPPORT.
- [Medium] cgroup_memory_max opens memory.max with plain O_WRONLY (line
  217) and writes an 8-page limit (line 225) to the cgroup that holds…
- [Medium] preserved_after_traffic checks `get_fwd_alloc(cfd) >= 16*ps`
  (line 325) right after recv(sfd, MSG_WAITALL) returns (line 323).
- [Low] The fallback `#define SO_RESERVE_MEM 73` in so_reserve_mem.c
  (line 21) is right only for the asm-generic, alpha and mips ABIs.
- [Low] FIXTURE_SETUP writes "+memory" to <cgroup2
  mount>/cgroup.subtree_control (line 80), and FIXTURE_TEARDOWN (lines…

--

selftests/net: add SO_RESERVE_MEM test

This commit adds a kselftest for SO_RESERVE_MEM. It covers rejection of
non-TCP sockets, bounds checks, reserve grow/shrink/rounding with the
matching SO_MEMINFO forward alloc, cgroup v2 memory.max enforcement, the
zero reserve on accepted children, and reserve retention across TCP
traffic.

> diff --git a/tools/testing/selftests/net/so_reserve_mem.c b/tools/testing/selftests/net/so_reserve_mem.c
> new file mode 100644
> index 0000000000000..c2ca02ebe8353
> --- /dev/null
> +++ b/tools/testing/selftests/net/so_reserve_mem.c
> @@ -0,0 +1,333 @@

[ ... ]

> +#ifndef SO_RESERVE_MEM
> +#define SO_RESERVE_MEM 73
> +#endif

[Severity: Low]
This isn't a bug in normal builds, because the Makefile pulls in the
kernel uapi headers and this fallback goes unused. Is 73 the right
fallback on every architecture, though?

parisc uses a different value:

arch/parisc/include/uapi/asm/socket.h:
#define SO_RESERVE_MEM		0x4047

So does sparc, where 73 (0x0049) is SO_BUSY_POLL_BUDGET:

arch/sparc/include/uapi/asm/socket.h:
#define SO_RESERVE_MEM           0x0052

If the fallback ever gets used on those architectures, the test would get
or set an unrelated socket option.

[ ... ]

> +FIXTURE_SETUP(so_reserve_mem)
> +{

[ ... ]

> +	snprintf(ctrl_path, sizeof(ctrl_path), "%s/cgroup.subtree_control",
> +		 self->cg_root);
> +	fd = open(ctrl_path, O_WRONLY);
> +	if (fd >= 0) {
> +		ret = write(fd, "+memory", 7);
> +		(void)ret;
> +		close(fd);
> +	}

[Severity: Medium]
Should the fixture skip when memcg socket accounting isn't available?

The result of the "+memory" write is thrown away. Nothing checks that the
child cgroup ends up with a non-root memcg. Without one,
sock_reserve_memory() rejects the request immediately:

net/core/sock.c:sock_reserve_memory() {
	...
	if (!mem_cgroup_sk_enabled(sk) || !sk_is_tcp(sk))
		return -EOPNOTSUPP;
	...
}

mem_cgroup_sk_enabled() is false in these cases:

  - CONFIG_MEMCG=n
  - booted with cgroup_disable=memory or cgroup.memory=nosocket
  - mem_cgroup_sk_alloc() left sk_memcg NULL (root memcg, or memcg bound
    to v1)

tools/testing/selftests/net/config also isn't updated with CONFIG_MEMCG or
CONFIG_CGROUPS.

On those kernels, the ASSERT_EQ(setsockopt(SO_RESERVE_MEM), 0) calls in
grow_shrink_and_rounding, accept_child_zero_reserve and
preserved_after_traffic fail with EOPNOTSUPP instead of skipping.

With cgroup.memory=nosocket, memory.max still exists, so cgroup_memory_max
fails too: errno is EOPNOTSUPP rather than ENOMEM.

In the same setups, non_tcp_rejected passes on the memcg check alone and
never reaches the sk_is_tcp() gate, because both checks return EOPNOTSUPP.
Would a positive TCP control be needed there to show that the TCP-only
restriction works?

[ ... ]

> +FIXTURE_TEARDOWN(so_reserve_mem)
> +{

[ ... ]

> +	rmdir(self->cg_child);
> +	umount2(self->cg_root, MNT_DETACH);
> +	rmdir(self->cg_root);
> +}

[Severity: Low]
Should teardown restore cgroup.subtree_control?

The mount namespace is private, but a cgroup2 mount always exposes the
single global v2 hierarchy. That makes cg_root the host's root cgroup (or
the cgroupns root).

FIXTURE_SETUP writes "+memory" there. Teardown never records the earlier
state and never writes "-memory".

On hosts or minimal CI VMs where the root subtree_control starts empty, the
memory controller stays enabled for all child cgroups after the test. That
changes memcg and socket memory accounting for later tests and workloads.

> +static void check_non_tcp_rejected(struct __test_metadata *_metadata,
> +				   int domain, int type, int protocol,
> +				   int val)
> +{
> +	int fd = socket(domain, type, protocol);
> +
> +	if (fd < 0) {
> +		EXPECT_TRUE(errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT);
> +		return;
> +	}

[Severity: Medium]
Should ENOPROTOOPT be accepted here as well?

MPTCP can be built in but disabled with net.mptcp.enabled=0. In that case
socket creation fails here:

net/mptcp/protocol.c:mptcp_init_sock() {
	...
	if (!mptcp_is_enabled(net))
		return -ENOPROTOOPT;
	...
}

inet_create() passes that error back to userspace. As a result,
socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP) in non_tcp_rejected fails with
ENOPROTOOPT, and the EXPECT_TRUE fails the test.

The test doesn't unshare the network namespace, so it sees the host's
sysctl. Any other environment-specific socket() errno also becomes an
EXPECT failure rather than a skip.

[ ... ]

> +TEST_F(so_reserve_mem, cgroup_memory_max)
> +{
> +	char max_path[160];
> +	int ps = self->page_size;
> +	int fd, max_fd, val;
> +
> +	snprintf(max_path, sizeof(max_path), "%s/memory.max", self->cg_child);
> +	max_fd = open(max_path, O_WRONLY);
> +	if (max_fd < 0)
> +		SKIP(return, "cgroup memory controller not delegated");
> +
> +	fd = socket(AF_INET, SOCK_STREAM, 0);
> +	ASSERT_GE(fd, 0);
> +
> +	/* Limit cgroup memory to 8 pages and try to reserve 64 pages */
> +	ASSERT_GT(dprintf(max_fd, "%ld\n", 8L * ps), 0);

[Severity: Medium]
Can this write OOM-kill the test process?

FIXTURE_SETUP moved the test process into cg_child, so this 8-page limit
applies to the test itself, and memory.current is never checked.

memory.max is opened without O_NONBLOCK, so memory_max_write() enforces
the new limit synchronously:

mm/memcontrol.c:memory_max_write() {
	...
	if (of->file->f_flags & O_NONBLOCK)
		goto out;

	for (;;) {
		...
		if (nr_pages <= max)
			break;
		...
		memcg_memory_event(memcg, MEMCG_OOM);
		if (!mem_cgroup_out_of_memory(memcg, GFP_KERNEL, 0))
			break;
		...
	}
	...
}

The process's charges after migration include accounted slab objects (the
socket, inode, dentry, file and seq_file) and any anon or CoW pages. Anon
pages can't be reclaimed without swap. If those charges stay above 8 pages
after reclaim, the memcg OOM killer kills the test during this write. It
then dies from a signal instead of seeing ENOMEM from setsockopt.

Usage is often only a few pages here. It depends on runtime details such as
glibc behaviour, THP on fresh faults, and LSM blobs.

Would opening memory.max with O_NONBLOCK, or setting the limit relative to
memory.current, avoid this?

[ ... ]

> +TEST_F(so_reserve_mem, preserved_after_traffic)
> +{

[ ... ]

> +	/* Send & drain traffic; cfd must retain its 16-page forward alloc,
> +	 * while sfd (0 reserve) reclaims its forward alloc back to 0.
> +	 */
> +	ASSERT_EQ(send(cfd, buf, sizeof(buf), 0), (ssize_t)sizeof(buf));
> +	ASSERT_EQ(recv(sfd, buf, sizeof(buf), MSG_WAITALL), (ssize_t)sizeof(buf));
> +
> +	EXPECT_GE(get_fwd_alloc(_metadata, cfd), (__u32)(16 * ps));

[Severity: Medium]
Is this check racing with the ACK being processed on cfd?

send() charges the queued skb to cfd. That leaves sk_forward_alloc at
16 * ps - truesize, with sk_wmem_queued equal to truesize.

sk_unused_reserved_mem() subtracts sk_wmem_queued. Forward alloc only
returns to 16 * ps once the skb is freed on ACK, followed by
sk_mem_uncharge() and sk_mem_reclaim().

On default non-RT kernels, the loopback softirq runs in the sender's
local_bh_enable(). The ACK lands on cfd's backlog and release_sock()
processes it before send() returns, so the check passes there.

With CONFIG_PREEMPT_RT, use_backlog_threads() always returns true. The
same happens with thread_backlog_napi or RPS on lo. RX then runs in a
separate backlog NAPI thread:

backlog thread
tcp_rcv_established()      (sfd)
  sends the ACK to cfd
  tcp_data_ready()         <- wakes the reader

test thread
recv(sfd) returns
get_fwd_alloc(cfd)         <- SO_MEMINFO, no socket lock taken

If the reader samples cfd before the backlog thread handles the ACK, it
sees fwd_alloc < 16 * ps and the EXPECT_GE fails intermittently.

The commit message says:

  - Verify reserve retention across TCP send/receive/ACK cycles.

However, nothing here waits for the ACK to complete. Would polling until
SK_MEMINFO_WMEM_QUEUED (or tcpi_unacked) reaches 0 before this check make
it reliable?

> +	EXPECT_EQ(get_fwd_alloc(_metadata, sfd), 0U);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924123602.1979090-1-edumazet%40google.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test
  2026-09-25 12:49   ` netdev-bot+sashiko
@ 2026-09-25 13:06     ` Eric Dumazet
  2026-09-25 13:21       ` Eric Dumazet
  2026-09-25 13:49     ` Matthieu Baerts
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2026-09-25 13:06 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, kuba, pabeni, weiwan, horms, caixinchen1, netdev, edumazet

On Fri, Sep 25, 2026 at 2:49 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 4 · Low: 2
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The new SO_RESERVE_MEM selftest needs memcg socket accounting
>   but never checks for it, so on systems without it the tests fail…
> - [Medium] When socket() fails, check_non_tcp_rejected() only accepts
>   EAFNOSUPPORT or EPROTONOSUPPORT.
> - [Medium] cgroup_memory_max opens memory.max with plain O_WRONLY (line
>   217) and writes an 8-page limit (line 225) to the cgroup that holds…
> - [Medium] preserved_after_traffic checks `get_fwd_alloc(cfd) >= 16*ps`
>   (line 325) right after recv(sfd, MSG_WAITALL) returns (line 323).
> - [Low] The fallback `#define SO_RESERVE_MEM 73` in so_reserve_mem.c
>   (line 21) is right only for the asm-generic, alpha and mips ABIs.
> - [Low] FIXTURE_SETUP writes "+memory" to <cgroup2
>   mount>/cgroup.subtree_control (line 80), and FIXTURE_TEARDOWN (lines…

I have no intention to work more  on this test, which was really a first step.

This is going to be 10 versions of AI agents trying to talk to each other.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test
  2026-09-25 13:06     ` Eric Dumazet
@ 2026-09-25 13:21       ` Eric Dumazet
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-09-25 13:21 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, kuba, pabeni, weiwan, horms, caixinchen1, netdev, edumazet

On Fri, Sep 25, 2026 at 3:06 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Sep 25, 2026 at 2:49 PM <netdev-bot+sashiko@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 6 potential
> > issue(s) to consider.
> >
> > Critical: 0 · High: 0 · Medium: 4 · Low: 2
> >
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> >
> > - [Medium] The new SO_RESERVE_MEM selftest needs memcg socket accounting
> >   but never checks for it, so on systems without it the tests fail…
> > - [Medium] When socket() fails, check_non_tcp_rejected() only accepts
> >   EAFNOSUPPORT or EPROTONOSUPPORT.
> > - [Medium] cgroup_memory_max opens memory.max with plain O_WRONLY (line
> >   217) and writes an 8-page limit (line 225) to the cgroup that holds…
> > - [Medium] preserved_after_traffic checks `get_fwd_alloc(cfd) >= 16*ps`
> >   (line 325) right after recv(sfd, MSG_WAITALL) returns (line 323).
> > - [Low] The fallback `#define SO_RESERVE_MEM 73` in so_reserve_mem.c
> >   (line 21) is right only for the asm-generic, alpha and mips ABIs.
> > - [Low] FIXTURE_SETUP writes "+memory" to <cgroup2
> >   mount>/cgroup.subtree_control (line 80), and FIXTURE_TEARDOWN (lines…
>
> I have no intention to work more  on this test, which was really a first step.
>
> This is going to be 10 versions of AI agents trying to talk to each other.

I will try a V2, but this will be the last version.

pw-bot: cr

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test
  2026-09-25 12:49   ` netdev-bot+sashiko
  2026-09-25 13:06     ` Eric Dumazet
@ 2026-09-25 13:49     ` Matthieu Baerts
  1 sibling, 0 replies; 9+ messages in thread
From: Matthieu Baerts @ 2026-09-25 13:49 UTC (permalink / raw)
  To: netdev-bot+sashiko, edumazet
  Cc: davem, kuba, pabeni, weiwan, horms, caixinchen1, netdev, edumazet

Hi Eric,

Thank you for the new test.

On 25/09/2026 14:49, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider.

(...)
>> +static void check_non_tcp_rejected(struct __test_metadata *_metadata,
>> +				   int domain, int type, int protocol,
>> +				   int val)
>> +{
>> +	int fd = socket(domain, type, protocol);
>> +
>> +	if (fd < 0) {
>> +		EXPECT_TRUE(errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT);
>> +		return;
>> +	}
> 
> [Severity: Medium]
> Should ENOPROTOOPT be accepted here as well?
> 
> MPTCP can be built in but disabled with net.mptcp.enabled=0. In that case
> socket creation fails here:
> 
> net/mptcp/protocol.c:mptcp_init_sock() {
> 	...
> 	if (!mptcp_is_enabled(net))
> 		return -ENOPROTOOPT;
> 	...
> }
> 
> inet_create() passes that error back to userspace. As a result,
> socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP) in non_tcp_rejected fails with
> ENOPROTOOPT, and the EXPECT_TRUE fails the test.
> 
> The test doesn't unshare the network namespace, so it sees the host's
> sysctl. Any other environment-specific socket() errno also becomes an
> EXPECT failure rather than a skip.
Typically, we try to run the tests in a dedicated netns to avoid such
issues. (We also force net.mptcp.enabled=1 because it is still disabled
on RHEL, which is really a shame... but I don't think we really need to
support that here in the selftests.)

At the end, I don't think we need to care about all possible cases, but
using a netns is probably a good idea, at least not to "poison" other
tests. (If that's easy to put in place.)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-25 13:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 12:36 [PATCH net 0/2] net: fix SO_RESERVE_MEM socket type restriction and value bounds Eric Dumazet
2026-09-24 12:36 ` [PATCH net 1/2] net: restrict SO_RESERVE_MEM to TCP sockets and cap max value Eric Dumazet
2026-09-24 17:32   ` Wei Wang
2026-09-25 12:49   ` netdev-bot+sashiko
2026-09-24 12:36 ` [PATCH net 2/2] selftests/net: add SO_RESERVE_MEM test Eric Dumazet
2026-09-25 12:49   ` netdev-bot+sashiko
2026-09-25 13:06     ` Eric Dumazet
2026-09-25 13:21       ` Eric Dumazet
2026-09-25 13:49     ` Matthieu Baerts

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox