lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 23 Dec 2022 08:54:32 +0900 From: Kuniyuki Iwashima <kuniyu@...zon.com> To: <joannelkoong@...il.com> CC: <davem@...emloft.net>, <edumazet@...gle.com>, <jirislaby@...nel.org>, <kuba@...nel.org>, <kuni1840@...il.com>, <kuniyu@...zon.com>, <netdev@...r.kernel.org>, <pabeni@...hat.com> Subject: Re: [PATCH RFC net 2/2] tcp: Add selftest for bind() and TIME_WAIT. From: Joanne Koong <joannelkoong@...il.com> Date: Thu, 22 Dec 2022 13:41:11 -0800 > On Wed, Dec 21, 2022 at 7:14 AM Kuniyuki Iwashima <kuniyu@...zon.com> wrote: > > > > bhash2 split the bind() validation logic into wildcard and non-wildcard > > cases. Let's add a test to catch the same regression. > > > > Before the previous patch: > > > > # ./bind_timewait > > TAP version 13 > > 1..2 > > # Starting 2 tests from 3 test cases. > > # RUN bind_timewait.localhost.1 ... > > # bind_timewait.c:87:1:Expected ret (0) == -1 (-1) > > # 1: Test terminated by assertion > > # FAIL bind_timewait.localhost.1 > > not ok 1 bind_timewait.localhost.1 > > # RUN bind_timewait.addrany.1 ... > > # OK bind_timewait.addrany.1 > > ok 2 bind_timewait.addrany.1 > > # FAILED: 1 / 2 tests passed. > > # Totals: pass:1 fail:1 xfail:0 xpass:0 skip:0 error:0 > > > > After: > > > > # ./bind_timewait > > TAP version 13 > > 1..2 > > # Starting 2 tests from 3 test cases. > > # RUN bind_timewait.localhost.1 ... > > # OK bind_timewait.localhost.1 > > ok 1 bind_timewait.localhost.1 > > # RUN bind_timewait.addrany.1 ... > > # OK bind_timewait.addrany.1 > > ok 2 bind_timewait.addrany.1 > > # PASSED: 2 / 2 tests passed. > > # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0 > > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com> > > --- > > tools/testing/selftests/net/.gitignore | 1 + > > tools/testing/selftests/net/bind_timewait.c | 93 +++++++++++++++++++++ > > 2 files changed, 94 insertions(+) > > create mode 100644 tools/testing/selftests/net/bind_timewait.c > > > > diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore > > index 9cc84114741d..a6911cae368c 100644 > > --- a/tools/testing/selftests/net/.gitignore > > +++ b/tools/testing/selftests/net/.gitignore > > @@ -1,5 +1,6 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > bind_bhash > > +bind_timewait > > csum > > cmsg_sender > > diag_uid > > diff --git a/tools/testing/selftests/net/bind_timewait.c b/tools/testing/selftests/net/bind_timewait.c > > new file mode 100644 > > index 000000000000..2d40403128ff > > --- /dev/null > > +++ b/tools/testing/selftests/net/bind_timewait.c > > @@ -0,0 +1,93 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright Amazon.com Inc. or its affiliates. */ > > + > > +#include "../kselftest_harness.h" > > nit: Not sure if this matters or not, but from looking at the other > selftests/net it seems like the convention is to have relative path > #include defined below absolute path #includes. Will fix. > > > + > > +#include <sys/socket.h> > > +#include <netinet/in.h> > > +#include <netinet/tcp.h> > > nit: i don't think we need this netinet/tcp.h include Good catch, I was seeing an old man page. > > > + > > +FIXTURE(bind_timewait) > > +{ > > + struct sockaddr_in addr; > > + socklen_t addrlen; > > +}; > > + > > +FIXTURE_VARIANT(bind_timewait) > > +{ > > + __u32 addr_const; > > +}; > > + > > +FIXTURE_VARIANT_ADD(bind_timewait, localhost) > > +{ > > + .addr_const = INADDR_LOOPBACK > > +}; > > + > > +FIXTURE_VARIANT_ADD(bind_timewait, addrany) > > +{ > > + .addr_const = INADDR_ANY > > +}; > > + > > +FIXTURE_SETUP(bind_timewait) > > +{ > > + self->addr.sin_family = AF_INET; > > + self->addr.sin_port = 0; > > + self->addr.sin_addr.s_addr = htonl(variant->addr_const); > > + self->addrlen = sizeof(self->addr); > > +} > > + > > +FIXTURE_TEARDOWN(bind_timewait) > > +{ > > +} > > + > > +void create_timewait_socket(struct __test_metadata *_metadata, > > + FIXTURE_DATA(bind_timewait) *self) > > +{ > > + int server_fd, client_fd, child_fd, ret; > > + struct sockaddr_in addr; > > + socklen_t addrlen; > > + > > + server_fd = socket(AF_INET, SOCK_STREAM, 0); > > + ASSERT_GT(server_fd, 0); > > If any of these assertions fail, do we leak fds because we don't get > to calling the close()s at the end of this function? Do we need to > have the fds cleaned up in the teardown fixture function? I think exit() cleans up fds in the case. IIUC, the parent process catches SIGABRT in __wait_for_test(), but the child does not call FIXTURE_TEARDOWN(). Thank you! > > > + > > + ret = bind(server_fd, (struct sockaddr *)&self->addr, self->addrlen); > > + ASSERT_EQ(ret, 0); > > + > > + ret = listen(server_fd, 1); > > + ASSERT_EQ(ret, 0); > > + > > + ret = getsockname(server_fd, (struct sockaddr *)&self->addr, &self->addrlen); > > + ASSERT_EQ(ret, 0); > > + > > + client_fd = socket(AF_INET, SOCK_STREAM, 0); > > + ASSERT_GT(client_fd, 0); > > + > > + ret = connect(client_fd, (struct sockaddr *)&self->addr, self->addrlen); > > + ASSERT_EQ(ret, 0); > > + > > + addrlen = sizeof(addr); > > + child_fd = accept(server_fd, (struct sockaddr *)&addr, &addrlen); > > + ASSERT_GT(child_fd, 0); > > + > > + close(child_fd); > > + close(client_fd); > > + close(server_fd); > > +} > > + > > +TEST_F(bind_timewait, 1) > > +{ > > + int fd, ret; > > + > > + create_timewait_socket(_metadata, self); > > + > > + fd = socket(AF_INET, SOCK_STREAM, 0); > > + ASSERT_GT(fd, 0); > > + > > + ret = bind(fd, (struct sockaddr *)&self->addr, self->addrlen); > > + ASSERT_EQ(ret, -1); > > + ASSERT_EQ(errno, EADDRINUSE); > > + > > + close(fd); > > +} > > + > > +TEST_HARNESS_MAIN > > -- > > 2.30.2
Powered by blists - more mailing lists