Re: [PATCH v3 net-next 9/9] selftest: af_unix: Test SO_PASSRIGHTS.

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

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@amazon.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
>