* [PATCH v4 net 1/3] udp: Fix multiple wraparounds of sk->sk_rmem_alloc.
2025-03-29 18:05 [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX Kuniyuki Iwashima
@ 2025-03-29 18:05 ` Kuniyuki Iwashima
2025-03-29 18:05 ` [PATCH v4 net 2/3] udp: Fix memory accounting leak Kuniyuki Iwashima
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Kuniyuki Iwashima @ 2025-03-29 18:05 UTC (permalink / raw)
To: Willem de Bruijn, David S. Miller, David Ahern, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
Willem de Bruijn
__udp_enqueue_schedule_skb() has the following condition:
if (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf)
goto drop;
sk->sk_rcvbuf is initialised by net.core.rmem_default and later can
be configured by SO_RCVBUF, which is limited by net.core.rmem_max,
or SO_RCVBUFFORCE.
If we set INT_MAX to sk->sk_rcvbuf, the condition is always false
as sk->sk_rmem_alloc is also signed int.
Then, the size of the incoming skb is added to sk->sk_rmem_alloc
unconditionally.
This results in integer overflow (possibly multiple times) on
sk->sk_rmem_alloc and allows a single socket to have skb up to
net.core.udp_mem[1].
For example, if we set a large value to udp_mem[1] and INT_MAX to
sk->sk_rcvbuf and flood packets to the socket, we can see multiple
overflows:
# cat /proc/net/sockstat | grep UDP:
UDP: inuse 3 mem 7956736 <-- (7956736 << 12) bytes > INT_MAX * 15
^- PAGE_SHIFT
# ss -uam
State Recv-Q ...
UNCONN -1757018048 ... <-- flipping the sign repeatedly
skmem:(r2537949248,rb2147483646,t0,tb212992,f1984,w0,o0,bl0,d0)
Previously, we had a boundary check for INT_MAX, which was removed by
commit 6a1f12dd85a8 ("udp: relax atomic operation on sk->sk_rmem_alloc").
A complete fix would be to revert it and cap the right operand by
INT_MAX:
rmem = atomic_add_return(size, &sk->sk_rmem_alloc);
if (rmem > min(size + (unsigned int)sk->sk_rcvbuf, INT_MAX))
goto uncharge_drop;
but we do not want to add the expensive atomic_add_return() back just
for the corner case.
Casting rmem to unsigned int prevents multiple wraparounds, but we still
allow a single wraparound.
# cat /proc/net/sockstat | grep UDP:
UDP: inuse 3 mem 524288 <-- (INT_MAX + 1) >> 12
# ss -uam
State Recv-Q ...
UNCONN -2147482816 ... <-- INT_MAX + 831 bytes
skmem:(r2147484480,rb2147483646,t0,tb212992,f3264,w0,o0,bl0,d14468947)
So, let's define rmem and rcvbuf as unsigned int and check skb->truesize
only when rcvbuf is large enough to lower the overflow possibility.
Note that we still have a small chance to see overflow if multiple skbs
to the same socket are processed on different core at the same time and
each size does not exceed the limit but the total size does.
Note also that we must ignore skb->truesize for a small buffer as
explained in commit 363dc73acacb ("udp: be less conservative with
sock rmem accounting").
Fixes: 6a1f12dd85a8 ("udp: relax atomic operation on sk->sk_rmem_alloc")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
v2:
* Define rmem and rcvbuf as unsigned int
* Take skb->truesize into account for the large rcvbuf case
---
net/ipv4/udp.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d0bffcfa56d8..07aa5db78c55 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1725,17 +1725,25 @@ static int udp_rmem_schedule(struct sock *sk, int size)
int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
{
struct sk_buff_head *list = &sk->sk_receive_queue;
- int rmem, err = -ENOMEM;
+ unsigned int rmem, rcvbuf;
spinlock_t *busy = NULL;
- int size, rcvbuf;
+ int size, err = -ENOMEM;
- /* Immediately drop when the receive queue is full.
- * Always allow at least one packet.
- */
rmem = atomic_read(&sk->sk_rmem_alloc);
rcvbuf = READ_ONCE(sk->sk_rcvbuf);
- if (rmem > rcvbuf)
- goto drop;
+ size = skb->truesize;
+
+ /* Immediately drop when the receive queue is full.
+ * Cast to unsigned int performs the boundary check for INT_MAX.
+ */
+ if (rmem + size > rcvbuf) {
+ if (rcvbuf > INT_MAX >> 1)
+ goto drop;
+
+ /* Always allow at least one packet for small buffer. */
+ if (rmem > rcvbuf)
+ goto drop;
+ }
/* Under mem pressure, it might be helpful to help udp_recvmsg()
* having linear skbs :
@@ -1748,7 +1756,7 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
busy = busylock_acquire(sk);
}
- size = skb->truesize;
+
udp_set_dev_scratch(skb);
atomic_add(size, &sk->sk_rmem_alloc);
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 net 2/3] udp: Fix memory accounting leak.
2025-03-29 18:05 [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX Kuniyuki Iwashima
2025-03-29 18:05 ` [PATCH v4 net 1/3] udp: Fix multiple wraparounds of sk->sk_rmem_alloc Kuniyuki Iwashima
@ 2025-03-29 18:05 ` Kuniyuki Iwashima
2025-03-29 18:05 ` [PATCH v4 net 3/3] selftest: net: Check wraparounds for sk->sk_rmem_alloc Kuniyuki Iwashima
2025-03-31 13:51 ` [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX Jakub Kicinski
3 siblings, 0 replies; 12+ messages in thread
From: Kuniyuki Iwashima @ 2025-03-29 18:05 UTC (permalink / raw)
To: Willem de Bruijn, David S. Miller, David Ahern, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
Matt Dowling, Willem de Bruijn
Matt Dowling reported a weird UDP memory usage issue.
Under normal operation, the UDP memory usage reported in /proc/net/sockstat
remains close to zero. However, it occasionally spiked to 524,288 pages
and never dropped. Moreover, the value doubled when the application was
terminated. Finally, it caused intermittent packet drops.
We can reproduce the issue with the script below [0]:
1. /proc/net/sockstat reports 0 pages
# cat /proc/net/sockstat | grep UDP:
UDP: inuse 1 mem 0
2. Run the script till the report reaches 524,288
# python3 test.py & sleep 5
# cat /proc/net/sockstat | grep UDP:
UDP: inuse 3 mem 524288 <-- (INT_MAX + 1) >> PAGE_SHIFT
3. Kill the socket and confirm the number never drops
# pkill python3 && sleep 5
# cat /proc/net/sockstat | grep UDP:
UDP: inuse 1 mem 524288
4. (necessary since v6.0) Trigger proto_memory_pcpu_drain()
# python3 test.py & sleep 1 && pkill python3
5. The number doubles
# cat /proc/net/sockstat | grep UDP:
UDP: inuse 1 mem 1048577
The application set INT_MAX to SO_RCVBUF, which triggered an integer
overflow in udp_rmem_release().
When a socket is close()d, udp_destruct_common() purges its receive
queue and sums up skb->truesize in the queue. This total is calculated
and stored in a local unsigned integer variable.
The total size is then passed to udp_rmem_release() to adjust memory
accounting. However, because the function takes a signed integer
argument, the total size can wrap around, causing an overflow.
Then, the released amount is calculated as follows:
1) Add size to sk->sk_forward_alloc.
2) Round down sk->sk_forward_alloc to the nearest lower multiple of
PAGE_SIZE and assign it to amount.
3) Subtract amount from sk->sk_forward_alloc.
4) Pass amount >> PAGE_SHIFT to __sk_mem_reduce_allocated().
When the issue occurred, the total in udp_destruct_common() was 2147484480
(INT_MAX + 833), which was cast to -2147482816 in udp_rmem_release().
At 1) sk->sk_forward_alloc is changed from 3264 to -2147479552, and
2) sets -2147479552 to amount. 3) reverts the wraparound, so we don't
see a warning in inet_sock_destruct(). However, udp_memory_allocated
ends up doubling at 4).
Since commit 3cd3399dd7a8 ("net: implement per-cpu reserves for
memory_allocated"), memory usage no longer doubles immediately after
a socket is close()d because __sk_mem_reduce_allocated() caches the
amount in udp_memory_per_cpu_fw_alloc. However, the next time a UDP
socket receives a packet, the subtraction takes effect, causing UDP
memory usage to double.
This issue makes further memory allocation fail once the socket's
sk->sk_rmem_alloc exceeds net.ipv4.udp_rmem_min, resulting in packet
drops.
To prevent this issue, let's use unsigned int for the calculation and
call sk_forward_alloc_add() only once for the small delta.
Note that first_packet_length() also potentially has the same problem.
[0]:
from socket import *
SO_RCVBUFFORCE = 33
INT_MAX = (2 ** 31) - 1
s = socket(AF_INET, SOCK_DGRAM)
s.bind(('', 0))
s.setsockopt(SOL_SOCKET, SO_RCVBUFFORCE, INT_MAX)
c = socket(AF_INET, SOCK_DGRAM)
c.connect(s.getsockname())
data = b'a' * 100
while True:
c.send(data)
Fixes: f970bd9e3a06 ("udp: implement memory accounting helpers")
Reported-by: Matt Dowling <madowlin@amazon.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/udp.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 07aa5db78c55..b7d03b0b4a07 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1625,12 +1625,12 @@ static bool udp_skb_has_head_state(struct sk_buff *skb)
}
/* fully reclaim rmem/fwd memory allocated for skb */
-static void udp_rmem_release(struct sock *sk, int size, int partial,
- bool rx_queue_lock_held)
+static void udp_rmem_release(struct sock *sk, unsigned int size,
+ int partial, bool rx_queue_lock_held)
{
struct udp_sock *up = udp_sk(sk);
struct sk_buff_head *sk_queue;
- int amt;
+ unsigned int amt;
if (likely(partial)) {
up->forward_deficit += size;
@@ -1650,10 +1650,8 @@ static void udp_rmem_release(struct sock *sk, int size, int partial,
if (!rx_queue_lock_held)
spin_lock(&sk_queue->lock);
-
- sk_forward_alloc_add(sk, size);
- amt = (sk->sk_forward_alloc - partial) & ~(PAGE_SIZE - 1);
- sk_forward_alloc_add(sk, -amt);
+ amt = (size + sk->sk_forward_alloc - partial) & ~(PAGE_SIZE - 1);
+ sk_forward_alloc_add(sk, size - amt);
if (amt)
__sk_mem_reduce_allocated(sk, amt >> PAGE_SHIFT);
@@ -1843,7 +1841,7 @@ EXPORT_IPV6_MOD_GPL(skb_consume_udp);
static struct sk_buff *__first_packet_length(struct sock *sk,
struct sk_buff_head *rcvq,
- int *total)
+ unsigned int *total)
{
struct sk_buff *skb;
@@ -1876,8 +1874,8 @@ static int first_packet_length(struct sock *sk)
{
struct sk_buff_head *rcvq = &udp_sk(sk)->reader_queue;
struct sk_buff_head *sk_queue = &sk->sk_receive_queue;
+ unsigned int total = 0;
struct sk_buff *skb;
- int total = 0;
int res;
spin_lock_bh(&rcvq->lock);
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 net 3/3] selftest: net: Check wraparounds for sk->sk_rmem_alloc.
2025-03-29 18:05 [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX Kuniyuki Iwashima
2025-03-29 18:05 ` [PATCH v4 net 1/3] udp: Fix multiple wraparounds of sk->sk_rmem_alloc Kuniyuki Iwashima
2025-03-29 18:05 ` [PATCH v4 net 2/3] udp: Fix memory accounting leak Kuniyuki Iwashima
@ 2025-03-29 18:05 ` Kuniyuki Iwashima
2025-03-31 13:51 ` [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX Jakub Kicinski
3 siblings, 0 replies; 12+ messages in thread
From: Kuniyuki Iwashima @ 2025-03-29 18:05 UTC (permalink / raw)
To: Willem de Bruijn, David S. Miller, David Ahern, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
Willem de Bruijn
The test creates client and server sockets and sets INT_MAX to the
server's SO_RCVBUFFORCE.
Then, the client floods packets to the server until the UDP memory
usage reaches (INT_MAX + 1) >> PAGE_SHIFT.
Finally, both sockets are close()d, and the last assert makes sure
that the memory usage drops to 0.
If needed, we can extend the test later for other protocols.
Without patch 1:
# Starting 2 tests from 2 test cases.
# RUN so_rcvbuf.udp_ipv4.rmem_max ...
# so_rcvbuf.c:163:rmem_max:Expected pages (524800) <= *variant->max_pages (524288)
# rmem_max: Test terminated by assertion
# FAIL so_rcvbuf.udp_ipv4.rmem_max
not ok 1 so_rcvbuf.udp_ipv4.rmem_max
Without patch 2:
# RUN so_rcvbuf.udp_ipv4.rmem_max ...
# so_rcvbuf.c:170:rmem_max:max_pages: 524288
# so_rcvbuf.c:178:rmem_max:Expected get_prot_pages(_metadata, variant) (524288) == 0 (0)
# rmem_max: Test terminated by assertion
# FAIL so_rcvbuf.udp_ipv4.rmem_max
not ok 1 so_rcvbuf.udp_ipv4.rmem_max
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
v4: Wait RCU for at most 30 sec
v3: Rebase to the latest net.git
v2: Add some comments (Note with 1000 loops it didn't fail at ASSERT_LE)
---
tools/testing/selftests/net/.gitignore | 3 +-
tools/testing/selftests/net/Makefile | 2 +-
tools/testing/selftests/net/so_rcvbuf.c | 188 ++++++++++++++++++++++++
3 files changed, 191 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/net/so_rcvbuf.c
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 679542f565a4..972fb07730d2 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -42,8 +42,9 @@ sk_so_peek_off
socket
so_incoming_cpu
so_netns_cookie
-so_txtime
so_rcv_listener
+so_rcvbuf
+so_txtime
stress_reuseport_listen
tap
tcp_fastopen_backup_key
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 6d718b478ed8..393ffa1c417b 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -87,7 +87,7 @@ TEST_GEN_PROGS += sk_bind_sendto_listen
TEST_GEN_PROGS += sk_connect_zero_addr
TEST_GEN_PROGS += sk_so_peek_off
TEST_PROGS += test_ingress_egress_chaining.sh
-TEST_GEN_PROGS += so_incoming_cpu
+TEST_GEN_PROGS += so_incoming_cpu so_rcvbuf
TEST_PROGS += sctp_vrf.sh
TEST_GEN_FILES += sctp_hello
TEST_GEN_FILES += ip_local_port_range
diff --git a/tools/testing/selftests/net/so_rcvbuf.c b/tools/testing/selftests/net/so_rcvbuf.c
new file mode 100644
index 000000000000..1a593033e47b
--- /dev/null
+++ b/tools/testing/selftests/net/so_rcvbuf.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright Amazon.com Inc. or its affiliates. */
+
+#include <limits.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+static int udp_max_pages;
+
+static int udp_parse_pages(struct __test_metadata *_metadata,
+ char *line, int *pages)
+{
+ int ret, unused;
+
+ if (strncmp(line, "UDP:", 4))
+ return -1;
+
+ ret = sscanf(line + 4, " inuse %d mem %d", &unused, pages);
+ ASSERT_EQ(2, ret);
+
+ return 0;
+}
+
+FIXTURE(so_rcvbuf)
+{
+ union {
+ struct sockaddr addr;
+ struct sockaddr_in addr4;
+ struct sockaddr_in6 addr6;
+ };
+ socklen_t addrlen;
+ int server;
+ int client;
+};
+
+FIXTURE_VARIANT(so_rcvbuf)
+{
+ int family;
+ int type;
+ int protocol;
+ int *max_pages;
+ int (*parse_pages)(struct __test_metadata *_metadata,
+ char *line, int *pages);
+};
+
+FIXTURE_VARIANT_ADD(so_rcvbuf, udp_ipv4)
+{
+ .family = AF_INET,
+ .type = SOCK_DGRAM,
+ .protocol = 0,
+ .max_pages = &udp_max_pages,
+ .parse_pages = udp_parse_pages,
+};
+
+FIXTURE_VARIANT_ADD(so_rcvbuf, udp_ipv6)
+{
+ .family = AF_INET6,
+ .type = SOCK_DGRAM,
+ .protocol = 0,
+ .max_pages = &udp_max_pages,
+ .parse_pages = udp_parse_pages,
+};
+
+static int get_page_shift(void)
+{
+ int page_size = getpagesize();
+ int page_shift = 0;
+
+ while (page_size > 1) {
+ page_size >>= 1;
+ page_shift++;
+ }
+
+ return page_shift;
+}
+
+FIXTURE_SETUP(so_rcvbuf)
+{
+ self->addr.sa_family = variant->family;
+
+ if (variant->family == AF_INET)
+ self->addrlen = sizeof(struct sockaddr_in);
+ else
+ self->addrlen = sizeof(struct sockaddr_in6);
+
+ udp_max_pages = (INT_MAX + 1L) >> get_page_shift();
+}
+
+FIXTURE_TEARDOWN(so_rcvbuf)
+{
+}
+
+static void create_socketpair(struct __test_metadata *_metadata,
+ FIXTURE_DATA(so_rcvbuf) *self,
+ const FIXTURE_VARIANT(so_rcvbuf) *variant)
+{
+ int ret;
+
+ self->server = socket(variant->family, variant->type, variant->protocol);
+ ASSERT_NE(self->server, -1);
+
+ self->client = socket(variant->family, variant->type, variant->protocol);
+ ASSERT_NE(self->client, -1);
+
+ ret = bind(self->server, &self->addr, self->addrlen);
+ ASSERT_EQ(ret, 0);
+
+ ret = getsockname(self->server, &self->addr, &self->addrlen);
+ ASSERT_EQ(ret, 0);
+
+ ret = connect(self->client, &self->addr, self->addrlen);
+ ASSERT_EQ(ret, 0);
+}
+
+static int get_prot_pages(struct __test_metadata *_metadata,
+ const FIXTURE_VARIANT(so_rcvbuf) *variant)
+{
+ char *line = NULL;
+ size_t unused;
+ int pages = 0;
+ FILE *f;
+
+ f = fopen("/proc/net/sockstat", "r");
+ ASSERT_NE(NULL, f);
+
+ while (getline(&line, &unused, f) != -1)
+ if (!variant->parse_pages(_metadata, line, &pages))
+ break;
+
+ free(line);
+ fclose(f);
+
+ return pages;
+}
+
+TEST_F(so_rcvbuf, rmem_max)
+{
+ int ret, i, pages;
+ char buf[16] = {};
+
+ create_socketpair(_metadata, self, variant);
+
+ ret = setsockopt(self->server, SOL_SOCKET, SO_RCVBUFFORCE,
+ &(int){INT_MAX}, sizeof(int));
+ ASSERT_EQ(ret, 0);
+
+ pages = get_prot_pages(_metadata, variant);
+ ASSERT_EQ(pages, 0);
+
+ for (i = 1; ; i++) {
+ ret = send(self->client, buf, sizeof(buf), 0);
+ ASSERT_EQ(ret, sizeof(buf));
+
+ /* Make sure we don't stop at pages == (INT_MAX >> PAGE_SHIFT)
+ * in case ASSERT_LE() should fail.
+ */
+ if (i % 10000 == 0) {
+ pages = get_prot_pages(_metadata, variant);
+
+ /* sk_rmem_alloc wrapped around by >PAGE_SIZE ? */
+ ASSERT_LE(pages, *variant->max_pages);
+
+ if (pages == *variant->max_pages)
+ break;
+ }
+ }
+
+ TH_LOG("max_pages: %d", pages);
+
+ close(self->client);
+ close(self->server);
+
+ /* Give RCU a chance to call udp_destruct_common() */
+ for (i = 0; i < 30; i++) {
+ sleep(1);
+
+ pages = get_prot_pages(_metadata, variant);
+ if (!pages)
+ break;
+ }
+
+ ASSERT_EQ(pages, 0);
+}
+
+TEST_HARNESS_MAIN
--
2.48.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-03-29 18:05 [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX Kuniyuki Iwashima
` (2 preceding siblings ...)
2025-03-29 18:05 ` [PATCH v4 net 3/3] selftest: net: Check wraparounds for sk->sk_rmem_alloc Kuniyuki Iwashima
@ 2025-03-31 13:51 ` Jakub Kicinski
2025-03-31 18:21 ` Kuniyuki Iwashima
3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2025-03-31 13:51 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: Willem de Bruijn, David S. Miller, David Ahern, Eric Dumazet,
Paolo Abeni, Simon Horman, Kuniyuki Iwashima, netdev
On Sat, 29 Mar 2025 11:05:10 -0700 Kuniyuki Iwashima wrote:
> v4:
> * Patch 4
> * Wait RCU for at most 30 sec
The new test still doesn't pass
TAP version 13
1..1
# timeout set to 3600
# selftests: net: so_rcvbuf
# 0.00 [+0.00] TAP version 13
# 0.00 [+0.00] 1..2
# 0.00 [+0.00] # Starting 2 tests from 2 test cases.
# 0.00 [+0.00] # RUN so_rcvbuf.udp_ipv4.rmem_max ...
# 0.00 [+0.00] # so_rcvbuf.c:151:rmem_max:Expected pages (35) == 0 (0)
# 0.00 [+0.00] # rmem_max: Test terminated by assertion
# 0.00 [+0.00] # FAIL so_rcvbuf.udp_ipv4.rmem_max
# 0.00 [+0.00] not ok 1 so_rcvbuf.udp_ipv4.rmem_max
# 0.01 [+0.00] # RUN so_rcvbuf.udp_ipv6.rmem_max ...
# 0.01 [+0.00] # so_rcvbuf.c:151:rmem_max:Expected pages (35) == 0 (0)
# 0.01 [+0.00] # rmem_max: Test terminated by assertion
# 0.01 [+0.00] # FAIL so_rcvbuf.udp_ipv6.rmem_max
# 0.01 [+0.00] not ok 2 so_rcvbuf.udp_ipv6.rmem_max
# 0.01 [+0.00] # FAILED: 0 / 2 tests passed.
# 0.01 [+0.00] # Totals: pass:0 fail:2 xfail:0 xpass:0 skip:0 error:0
not ok 1 selftests: net: so_rcvbuf # exit=1
Plus we also see failures in udpgso.sh
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-03-31 13:51 ` [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX Jakub Kicinski
@ 2025-03-31 18:21 ` Kuniyuki Iwashima
2025-03-31 18:36 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Kuniyuki Iwashima @ 2025-03-31 18:21 UTC (permalink / raw)
To: kuba
Cc: davem, dsahern, edumazet, horms, kuni1840, kuniyu, netdev, pabeni,
willemdebruijn.kernel
From: Jakub Kicinski <kuba@kernel.org>
Date: Mon, 31 Mar 2025 06:51:09 -0700
> On Sat, 29 Mar 2025 11:05:10 -0700 Kuniyuki Iwashima wrote:
> > v4:
> > * Patch 4
> > * Wait RCU for at most 30 sec
>
> The new test still doesn't pass
>
> TAP version 13
> 1..1
> # timeout set to 3600
> # selftests: net: so_rcvbuf
> # 0.00 [+0.00] TAP version 13
> # 0.00 [+0.00] 1..2
> # 0.00 [+0.00] # Starting 2 tests from 2 test cases.
> # 0.00 [+0.00] # RUN so_rcvbuf.udp_ipv4.rmem_max ...
> # 0.00 [+0.00] # so_rcvbuf.c:151:rmem_max:Expected pages (35) == 0 (0)
> # 0.00 [+0.00] # rmem_max: Test terminated by assertion
> # 0.00 [+0.00] # FAIL so_rcvbuf.udp_ipv4.rmem_max
> # 0.00 [+0.00] not ok 1 so_rcvbuf.udp_ipv4.rmem_max
> # 0.01 [+0.00] # RUN so_rcvbuf.udp_ipv6.rmem_max ...
> # 0.01 [+0.00] # so_rcvbuf.c:151:rmem_max:Expected pages (35) == 0 (0)
> # 0.01 [+0.00] # rmem_max: Test terminated by assertion
> # 0.01 [+0.00] # FAIL so_rcvbuf.udp_ipv6.rmem_max
> # 0.01 [+0.00] not ok 2 so_rcvbuf.udp_ipv6.rmem_max
> # 0.01 [+0.00] # FAILED: 0 / 2 tests passed.
> # 0.01 [+0.00] # Totals: pass:0 fail:2 xfail:0 xpass:0 skip:0 error:0
> not ok 1 selftests: net: so_rcvbuf # exit=1
>
>
> Plus we also see failures in udpgso.sh
I forgot to update `size` when skb_condense() is called.
Without the change I saw the new test stuck at 1 page after
udpgso.sh, but with the change both passed.
Will post v5.
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-03-31 18:21 ` Kuniyuki Iwashima
@ 2025-03-31 18:36 ` Jakub Kicinski
2025-03-31 18:54 ` Kuniyuki Iwashima
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2025-03-31 18:36 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: davem, dsahern, edumazet, horms, kuni1840, netdev, pabeni,
willemdebruijn.kernel
On Mon, 31 Mar 2025 11:21:34 -0700 Kuniyuki Iwashima wrote:
> > Plus we also see failures in udpgso.sh
>
> I forgot to update `size` when skb_condense() is called.
>
> Without the change I saw the new test stuck at 1 page after
> udpgso.sh, but with the change both passed.
>
> Will post v5.
Please do test locally if you can.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-03-31 18:36 ` Jakub Kicinski
@ 2025-03-31 18:54 ` Kuniyuki Iwashima
2025-03-31 20:31 ` Kuniyuki Iwashima
0 siblings, 1 reply; 12+ messages in thread
From: Kuniyuki Iwashima @ 2025-03-31 18:54 UTC (permalink / raw)
To: kuba
Cc: davem, dsahern, edumazet, horms, kuni1840, kuniyu, netdev, pabeni,
willemdebruijn.kernel
From: Jakub Kicinski <kuba@kernel.org>
Date: Mon, 31 Mar 2025 11:36:03 -0700
> On Mon, 31 Mar 2025 11:21:34 -0700 Kuniyuki Iwashima wrote:
> > > Plus we also see failures in udpgso.sh
> >
> > I forgot to update `size` when skb_condense() is called.
> >
> > Without the change I saw the new test stuck at 1 page after
> > udpgso.sh, but with the change both passed.
> >
> > Will post v5.
>
> Please do test locally if you can.
Sure, will try the same tests with CI.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-03-31 18:54 ` Kuniyuki Iwashima
@ 2025-03-31 20:31 ` Kuniyuki Iwashima
2025-03-31 23:29 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Kuniyuki Iwashima @ 2025-03-31 20:31 UTC (permalink / raw)
To: kuniyu
Cc: davem, dsahern, edumazet, horms, kuba, kuni1840, netdev, pabeni,
willemdebruijn.kernel
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Mon, 31 Mar 2025 11:54:53 -0700
> From: Jakub Kicinski <kuba@kernel.org>
> Date: Mon, 31 Mar 2025 11:36:03 -0700
> > On Mon, 31 Mar 2025 11:21:34 -0700 Kuniyuki Iwashima wrote:
> > > > Plus we also see failures in udpgso.sh
> > >
> > > I forgot to update `size` when skb_condense() is called.
> > >
> > > Without the change I saw the new test stuck at 1 page after
> > > udpgso.sh, but with the change both passed.
> > >
> > > Will post v5.
> >
> > Please do test locally if you can.
>
> Sure, will try the same tests with CI.
Is there a way to tell NIPA to run a test in a dedicated VM ?
I see some tests succeed when executed solely but fail when
executed with
make -C tools/testing/selftests/ TARGETS=net run_tests
When combined with other tests, assuming that the global UDP usage
will soon drop to 0 is not always easy... so it's defeating the
purpose but I'd drop the test in v5 not to make CI unhappy.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-03-31 20:31 ` Kuniyuki Iwashima
@ 2025-03-31 23:29 ` Jakub Kicinski
2025-04-01 1:15 ` Willem de Bruijn
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2025-03-31 23:29 UTC (permalink / raw)
To: Kuniyuki Iwashima, willemdebruijn.kernel
Cc: davem, dsahern, edumazet, horms, kuni1840, netdev, pabeni
On Mon, 31 Mar 2025 13:31:47 -0700 Kuniyuki Iwashima wrote:
> > > Please do test locally if you can.
> >
> > Sure, will try the same tests with CI.
>
> Is there a way to tell NIPA to run a test in a dedicated VM ?
>
> I see some tests succeed when executed solely but fail when
> executed with
>
> make -C tools/testing/selftests/ TARGETS=net run_tests
>
> When combined with other tests, assuming that the global UDP usage
> will soon drop to 0 is not always easy... so it's defeating the
> purpose but I'd drop the test in v5 not to make CI unhappy.
Can we account for some level of system noise? Or try to dump all
the sockets and count the "accounted for" in-use memory?
We can do various things in NIPA, but I'm not sure if it's okay
for tests inside net/ should require a completely idle system.
If we want a completely idle system maybe user-mode Linux + kunit
is a better direction?
Willem, WDYT?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-03-31 23:29 ` Jakub Kicinski
@ 2025-04-01 1:15 ` Willem de Bruijn
2025-04-01 18:35 ` Kuniyuki Iwashima
0 siblings, 1 reply; 12+ messages in thread
From: Willem de Bruijn @ 2025-04-01 1:15 UTC (permalink / raw)
To: Jakub Kicinski, Kuniyuki Iwashima, willemdebruijn.kernel
Cc: davem, dsahern, edumazet, horms, kuni1840, netdev, pabeni
Jakub Kicinski wrote:
> On Mon, 31 Mar 2025 13:31:47 -0700 Kuniyuki Iwashima wrote:
> > > > Please do test locally if you can.
> > >
> > > Sure, will try the same tests with CI.
> >
> > Is there a way to tell NIPA to run a test in a dedicated VM ?
> >
> > I see some tests succeed when executed solely but fail when
> > executed with
> >
> > make -C tools/testing/selftests/ TARGETS=net run_tests
> >
> > When combined with other tests, assuming that the global UDP usage
> > will soon drop to 0 is not always easy... so it's defeating the
> > purpose but I'd drop the test in v5 not to make CI unhappy.
>
> Can we account for some level of system noise? Or try to dump all
> the sockets and count the "accounted for" in-use memory?
>
> We can do various things in NIPA, but I'm not sure if it's okay
> for tests inside net/ should require a completely idle system.
> If we want a completely idle system maybe user-mode Linux + kunit
> is a better direction?
>
> Willem, WDYT?
The number of tests depending on global variables like
proto_memory_allocated is thankfully low.
kselftest/runner.sh runs RUN_IN_NETNS tests in parallel. That sounds
the case here. Perhaps we can add a test option to force running
without concurrent other tests?
Otherwise, the specific test drops usage from MAX to 0. And verifies
to reach MAX before exiting its loop.
Other concurrent tests are unlikely to spike very high. It might just
be sufficient to relax that ASSERT to something a bit higher than 0,
but a far cry from INT_MAX, to mean "clearly no longer stressed".
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 net 0/3] udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX.
2025-04-01 1:15 ` Willem de Bruijn
@ 2025-04-01 18:35 ` Kuniyuki Iwashima
0 siblings, 0 replies; 12+ messages in thread
From: Kuniyuki Iwashima @ 2025-04-01 18:35 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: davem, dsahern, edumazet, horms, kuba, kuni1840, kuniyu, netdev,
pabeni
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Mon, 31 Mar 2025 21:15:24 -0400
> Jakub Kicinski wrote:
> > On Mon, 31 Mar 2025 13:31:47 -0700 Kuniyuki Iwashima wrote:
> > > > > Please do test locally if you can.
> > > >
> > > > Sure, will try the same tests with CI.
> > >
> > > Is there a way to tell NIPA to run a test in a dedicated VM ?
> > >
> > > I see some tests succeed when executed solely but fail when
> > > executed with
> > >
> > > make -C tools/testing/selftests/ TARGETS=net run_tests
> > >
> > > When combined with other tests, assuming that the global UDP usage
> > > will soon drop to 0 is not always easy... so it's defeating the
> > > purpose but I'd drop the test in v5 not to make CI unhappy.
> >
> > Can we account for some level of system noise? Or try to dump all
> > the sockets and count the "accounted for" in-use memory?
> >
> > We can do various things in NIPA, but I'm not sure if it's okay
> > for tests inside net/ should require a completely idle system.
> > If we want a completely idle system maybe user-mode Linux + kunit
> > is a better direction?
> >
> > Willem, WDYT?
>
> The number of tests depending on global variables like
> proto_memory_allocated is thankfully low.
>
> kselftest/runner.sh runs RUN_IN_NETNS tests in parallel. That sounds
> the case here. Perhaps we can add a test option to force running
> without concurrent other tests?
>
> Otherwise, the specific test drops usage from MAX to 0. And verifies
> to reach MAX before exiting its loop.
Yes, and we need to query rmem_alloc via netlink.
Also, I haven't investigated, but I saw a weird timeout, when the usage
stuck at 523914, which is smaller than INT_MAX >> PAGE_SHIFT.
Probably the test needs to check sockets' rmem_alloc to be accurate.
I'll drop the test for now and follow up in net-next.
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread