| Message ID | 20250323231016.74813-4-kuniyu@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | udp: Fix two integer overflows when sk->sk_rcvbuf is close to INT_MAX. | expand |
Kuniyuki Iwashima wrote: > 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: > > # RUN so_rcvbuf.udp_ipv4.rmem_max ... > # so_rcvbuf.c:160: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:167:rmem_max:max_pages: 524288 > # so_rcvbuf.c:175: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> > --- > tools/testing/selftests/net/.gitignore | 1 + > tools/testing/selftests/net/Makefile | 2 +- > tools/testing/selftests/net/so_rcvbuf.c | 178 ++++++++++++++++++++++++ > 3 files changed, 180 insertions(+), 1 deletion(-) > 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 28a715a8ef2b..befbdfb26581 100644 > --- a/tools/testing/selftests/net/.gitignore > +++ b/tools/testing/selftests/net/.gitignore > @@ -41,6 +41,7 @@ sk_so_peek_off > socket > so_incoming_cpu > so_netns_cookie > +so_rcvbuf > so_txtime > stress_reuseport_listen > tap > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile > index 8f32b4f01aee..d04428eaa819 100644 > --- a/tools/testing/selftests/net/Makefile > +++ b/tools/testing/selftests/net/Makefile > @@ -83,7 +83,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..6e20bafce32e > --- /dev/null > +++ b/tools/testing/selftests/net/so_rcvbuf.c > @@ -0,0 +1,178 @@ > +// 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) > +{ > + char buf[16] = {}; > + int ret, i; > + > + create_socketpair(_metadata, self, variant); > + > + ret = setsockopt(self->server, SOL_SOCKET, SO_RCVBUFFORCE, > + &(int){INT_MAX}, sizeof(int)); > + ASSERT_EQ(ret, 0); > + > + ASSERT_EQ(get_prot_pages(_metadata, variant), 0); > + > + for (i = 1; ; i++) { > + ret = send(self->client, buf, sizeof(buf), 0); > + ASSERT_EQ(ret, sizeof(buf)); > + > + if (i % 10000 == 0) { > + int pages = get_prot_pages(_metadata, variant); > + > + /* sk_rmem_alloc wrapped around too much ? */ > + ASSERT_LE(pages, *variant->max_pages); > + > + if (pages == *variant->max_pages) > + break; Does correctness depend here on max_pages being a multiple of 10K? > + } > + } > + > + TH_LOG("max_pages: %d", get_prot_pages(_metadata, variant)); > + > + close(self->client); > + close(self->server); > + > + /* Give RCU a chance to call udp_destruct_common() */ > + sleep(5); > + > + ASSERT_EQ(get_prot_pages(_metadata, variant), 0); > +} > + > +TEST_HARNESS_MAIN > -- > 2.48.1 >
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: Mon, 24 Mar 2025 11:58:06 -0400 > > +TEST_F(so_rcvbuf, rmem_max) > > +{ > > + char buf[16] = {}; > > + int ret, i; > > + > > + create_socketpair(_metadata, self, variant); > > + > > + ret = setsockopt(self->server, SOL_SOCKET, SO_RCVBUFFORCE, > > + &(int){INT_MAX}, sizeof(int)); > > + ASSERT_EQ(ret, 0); > > + > > + ASSERT_EQ(get_prot_pages(_metadata, variant), 0); > > + > > + for (i = 1; ; i++) { > > + ret = send(self->client, buf, sizeof(buf), 0); > > + ASSERT_EQ(ret, sizeof(buf)); > > + > > + if (i % 10000 == 0) { > > + int pages = get_prot_pages(_metadata, variant); > > + > > + /* sk_rmem_alloc wrapped around too much ? */ > > + ASSERT_LE(pages, *variant->max_pages); > > + > > + if (pages == *variant->max_pages) > > + break; > > Does correctness depend here on max_pages being a multiple of 10K? 10K may be too conservative, but at least we need to ensure that the size of accumulated skbs exceeds 1 PAGE_SIZE to fail on the ASSERT_LE(), otherwise we can't detect the multiple wraparounds even without patch 1. The later sleep for call_rcu() was dominant than this loop on my machine. Thanks!
Kuniyuki Iwashima wrote: > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > Date: Mon, 24 Mar 2025 11:58:06 -0400 > > > +TEST_F(so_rcvbuf, rmem_max) > > > +{ > > > + char buf[16] = {}; > > > + int ret, i; > > > + > > > + create_socketpair(_metadata, self, variant); > > > + > > > + ret = setsockopt(self->server, SOL_SOCKET, SO_RCVBUFFORCE, > > > + &(int){INT_MAX}, sizeof(int)); > > > + ASSERT_EQ(ret, 0); > > > + > > > + ASSERT_EQ(get_prot_pages(_metadata, variant), 0); > > > + > > > + for (i = 1; ; i++) { > > > + ret = send(self->client, buf, sizeof(buf), 0); > > > + ASSERT_EQ(ret, sizeof(buf)); > > > + > > > + if (i % 10000 == 0) { > > > + int pages = get_prot_pages(_metadata, variant); > > > + > > > + /* sk_rmem_alloc wrapped around too much ? */ > > > + ASSERT_LE(pages, *variant->max_pages); > > > + > > > + if (pages == *variant->max_pages) > > > + break; > > > > Does correctness depend here on max_pages being a multiple of 10K? > > 10K may be too conservative, but at least we need to ensure > that the size of accumulated skbs exceeds 1 PAGE_SIZE to > fail on the ASSERT_LE(), otherwise we can't detect the multiple > wraparounds even without patch 1. Thanks. It took me some time to understand. Without overflow, the pages counter will saturate at max_pages as the queue fills up. > The later sleep for call_rcu() was dominant than this loop on > my machine. Ack.
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore index 28a715a8ef2b..befbdfb26581 100644 --- a/tools/testing/selftests/net/.gitignore +++ b/tools/testing/selftests/net/.gitignore @@ -41,6 +41,7 @@ sk_so_peek_off socket so_incoming_cpu so_netns_cookie +so_rcvbuf so_txtime stress_reuseport_listen tap diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index 8f32b4f01aee..d04428eaa819 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -83,7 +83,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..6e20bafce32e --- /dev/null +++ b/tools/testing/selftests/net/so_rcvbuf.c @@ -0,0 +1,178 @@ +// 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) +{ + char buf[16] = {}; + int ret, i; + + create_socketpair(_metadata, self, variant); + + ret = setsockopt(self->server, SOL_SOCKET, SO_RCVBUFFORCE, + &(int){INT_MAX}, sizeof(int)); + ASSERT_EQ(ret, 0); + + ASSERT_EQ(get_prot_pages(_metadata, variant), 0); + + for (i = 1; ; i++) { + ret = send(self->client, buf, sizeof(buf), 0); + ASSERT_EQ(ret, sizeof(buf)); + + if (i % 10000 == 0) { + int pages = get_prot_pages(_metadata, variant); + + /* sk_rmem_alloc wrapped around too much ? */ + ASSERT_LE(pages, *variant->max_pages); + + if (pages == *variant->max_pages) + break; + } + } + + TH_LOG("max_pages: %d", get_prot_pages(_metadata, variant)); + + close(self->client); + close(self->server); + + /* Give RCU a chance to call udp_destruct_common() */ + sleep(5); + + ASSERT_EQ(get_prot_pages(_metadata, variant), 0); +} + +TEST_HARNESS_MAIN
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: # RUN so_rcvbuf.udp_ipv4.rmem_max ... # so_rcvbuf.c:160: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:167:rmem_max:max_pages: 524288 # so_rcvbuf.c:175: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> --- tools/testing/selftests/net/.gitignore | 1 + tools/testing/selftests/net/Makefile | 2 +- tools/testing/selftests/net/so_rcvbuf.c | 178 ++++++++++++++++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/net/so_rcvbuf.c