| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJnrk1Zc5Zz7c5CY8t14-Mg3SPmGFwCB6TFbPHfSSkexFJW8uw@mail.gmail.com>
Date: Thu, 22 Dec 2022 13:41:11 -0800
From: Joanne Koong <joannelkoong@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Jiri Slaby <jirislaby@...nel.org>,
Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH RFC net 2/2] tcp: Add selftest for bind() and TIME_WAIT.
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.
> +
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <netinet/tcp.h>
nit: i don't think we need this netinet/tcp.h include
> +
> +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?
> +
> + 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