[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6826401ac346c_25ebe5294c3@willemb.c.googlers.com.notmuch>
Date: Thu, 15 May 2025 15:27:22 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Willem de Bruijn <willemb@...gle.com>
Cc: Simon Horman <horms@...nel.org>,
Christian Brauner <brauner@...nel.org>,
Kuniyuki Iwashima <kuniyu@...zon.com>,
Kuniyuki Iwashima <kuni1840@...il.com>,
netdev@...r.kernel.org
Subject: Re: [PATCH v3 net-next 9/9] selftest: af_unix: Test SO_PASSRIGHTS.
Kuniyuki Iwashima wrote:
> scm_rights.c has various patterns of tests to exercise GC.
>
> Let's add cases where SO_PASSRIGHTS is disabled.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
> .../selftests/net/af_unix/scm_rights.c | 84 ++++++++++++++++++-
> 1 file changed, 81 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/net/af_unix/scm_rights.c b/tools/testing/selftests/net/af_unix/scm_rights.c
> index d66336256580..7589f690fe2f 100644
> --- a/tools/testing/selftests/net/af_unix/scm_rights.c
> +++ b/tools/testing/selftests/net/af_unix/scm_rights.c
> @@ -23,6 +23,7 @@ FIXTURE_VARIANT(scm_rights)
> int type;
> int flags;
> bool test_listener;
> + bool disabled;
> };
>
> FIXTURE_VARIANT_ADD(scm_rights, dgram)
> @@ -31,6 +32,16 @@ FIXTURE_VARIANT_ADD(scm_rights, dgram)
> .type = SOCK_DGRAM,
> .flags = 0,
> .test_listener = false,
> + .disabled = false,
> +};
> +
> +FIXTURE_VARIANT_ADD(scm_rights, dgram_disabled)
> +{
> + .name = "UNIX ",
> + .type = SOCK_DGRAM,
> + .flags = 0,
> + .test_listener = false,
> + .disabled = true,
> };
>
> FIXTURE_VARIANT_ADD(scm_rights, stream)
> @@ -39,6 +50,16 @@ FIXTURE_VARIANT_ADD(scm_rights, stream)
> .type = SOCK_STREAM,
> .flags = 0,
> .test_listener = false,
> + .disabled = false,
> +};
> +
> +FIXTURE_VARIANT_ADD(scm_rights, stream_disabled)
> +{
> + .name = "UNIX-STREAM ",
> + .type = SOCK_STREAM,
> + .flags = 0,
> + .test_listener = false,
> + .disabled = true,
> };
>
> FIXTURE_VARIANT_ADD(scm_rights, stream_oob)
> @@ -47,6 +68,16 @@ FIXTURE_VARIANT_ADD(scm_rights, stream_oob)
> .type = SOCK_STREAM,
> .flags = MSG_OOB,
> .test_listener = false,
> + .disabled = false,
> +};
> +
> +FIXTURE_VARIANT_ADD(scm_rights, stream_oob_disabled)
> +{
> + .name = "UNIX-STREAM ",
> + .type = SOCK_STREAM,
> + .flags = MSG_OOB,
> + .test_listener = false,
> + .disabled = true,
> };
>
> FIXTURE_VARIANT_ADD(scm_rights, stream_listener)
> @@ -55,6 +86,16 @@ FIXTURE_VARIANT_ADD(scm_rights, stream_listener)
> .type = SOCK_STREAM,
> .flags = 0,
> .test_listener = true,
> + .disabled = false,
> +};
> +
> +FIXTURE_VARIANT_ADD(scm_rights, stream_listener_disabled)
> +{
> + .name = "UNIX-STREAM ",
> + .type = SOCK_STREAM,
> + .flags = 0,
> + .test_listener = true,
> + .disabled = true,
> };
>
> FIXTURE_VARIANT_ADD(scm_rights, stream_listener_oob)
> @@ -63,6 +104,16 @@ FIXTURE_VARIANT_ADD(scm_rights, stream_listener_oob)
> .type = SOCK_STREAM,
> .flags = MSG_OOB,
> .test_listener = true,
> + .disabled = false,
> +};
> +
> +FIXTURE_VARIANT_ADD(scm_rights, stream_listener_oob_disabled)
> +{
> + .name = "UNIX-STREAM ",
> + .type = SOCK_STREAM,
> + .flags = MSG_OOB,
> + .test_listener = true,
> + .disabled = true,
> };
>
> static int count_sockets(struct __test_metadata *_metadata,
> @@ -105,6 +156,9 @@ FIXTURE_SETUP(scm_rights)
> ret = unshare(CLONE_NEWNET);
> ASSERT_EQ(0, ret);
>
> + if (variant->disabled)
> + return;
> +
> ret = count_sockets(_metadata, variant);
> ASSERT_EQ(0, ret);
> }
> @@ -113,6 +167,9 @@ FIXTURE_TEARDOWN(scm_rights)
> {
> int ret;
>
> + if (variant->disabled)
> + return;
> +
> sleep(1);
>
> ret = count_sockets(_metadata, variant);
> @@ -121,6 +178,7 @@ FIXTURE_TEARDOWN(scm_rights)
>
> static void create_listeners(struct __test_metadata *_metadata,
> FIXTURE_DATA(scm_rights) *self,
> + const FIXTURE_VARIANT(scm_rights) *variant,
> int n)
> {
> struct sockaddr_un addr = {
> @@ -140,6 +198,12 @@ static void create_listeners(struct __test_metadata *_metadata,
> ret = listen(self->fd[i], -1);
> ASSERT_EQ(0, ret);
>
> + if (variant->disabled) {
> + ret = setsockopt(self->fd[i], SOL_SOCKET, SO_PASSRIGHTS,
> + &(int){0}, sizeof(int));
> + ASSERT_EQ(0, ret);
> + }
> +
> addrlen = sizeof(addr);
> ret = getsockname(self->fd[i], (struct sockaddr *)&addr, &addrlen);
> ASSERT_EQ(0, ret);
> @@ -164,6 +228,12 @@ static void create_socketpairs(struct __test_metadata *_metadata,
> for (i = 0; i < n * 2; i += 2) {
> ret = socketpair(AF_UNIX, variant->type, 0, self->fd + i);
> ASSERT_EQ(0, ret);
> +
> + if (variant->disabled) {
> + ret = setsockopt(self->fd[i], SOL_SOCKET, SO_PASSRIGHTS,
> + &(int){0}, sizeof(int));
> + ASSERT_EQ(0, ret);
> + }
> }
> }
>
> @@ -175,7 +245,7 @@ static void __create_sockets(struct __test_metadata *_metadata,
> ASSERT_LE(n * 2, sizeof(self->fd) / sizeof(self->fd[0]));
>
> if (variant->test_listener)
> - create_listeners(_metadata, self, n);
> + create_listeners(_metadata, self, variant, n);
> else
> create_socketpairs(_metadata, self, variant, n);
> }
> @@ -227,10 +297,18 @@ void __send_fd(struct __test_metadata *_metadata,
> .msg_control = &cmsg,
> .msg_controllen = CMSG_SPACE(sizeof(cmsg.fd)),
> };
> - int ret;
> + int ret, saved_errno;
>
> + errno = 0;
> ret = sendmsg(self->fd[receiver * 2 + 1], &msg, variant->flags);
> - ASSERT_EQ(MSGLEN, ret);
> + saved_errno = errno;
> +
> + if (variant->disabled) {
> + ASSERT_EQ(-1, ret);
> + ASSERT_EQ(-EPERM, -saved_errno);
> + } else {
> + ASSERT_EQ(MSGLEN, ret);
> + }
Why is this errno complexity needed?
It should never be needed to manually reset errno.
Is the saved_errno there because ASSERT_EQ on ret could call a libc
function that resets errno?
> }
>
> #define create_sockets(n) \
> --
> 2.49.0
>
Powered by blists - more mailing lists