linux-kernel - Re: [PATCH v9 3/5] selftests/Landlock: Adding pathname Unix socket tests

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240819.Ohph6ahphohH@digikod.net>
Date: Mon, 19 Aug 2024 21:47:23 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: Tahera Fahimi <fahimitahera@...il.com>
Cc: outreachy@...ts.linux.dev, gnoack@...gle.com, paul@...l-moore.com, 
	jmorris@...ei.org, serge@...lyn.com, linux-security-module@...r.kernel.org, 
	linux-kernel@...r.kernel.org, bjorn3_gh@...tonmail.com, jannh@...gle.com, 
	netdev@...r.kernel.org
Subject: Re: [PATCH v9 3/5] selftests/Landlock: Adding pathname Unix socket
 tests

On Wed, Aug 14, 2024 at 12:22:21AM -0600, Tahera Fahimi wrote:
> This patch expands abstract Unix socket restriction tests by
> testing pathname sockets connection with scoped domain.
> 
> pathname_address_sockets ensures that Unix sockets bound to
> a null-terminated filesystem can still connect to a socket

"bound to a filesystem path name"

> outside of their scoped domain. This means that even if the
> domain is scoped with LANDLOCK_SCOPED_ABSTRACT_UNIX_SOCKET,
> the socket can connect to a socket outside the scoped domain.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@...il.com>
> ---
> changes in versions:
> v9:
> - Moving remove_path() back to fs_test.c, and using unlink(2)
>   and rmdir(2) instead.
> - Removing hard-coded numbers and using "backlog" instead.
> V8:
> - Adding pathname_address_sockets to cover all types of address
>   formats for unix sockets, and moving remove_path() to
>   common.h to reuse in this test.
> ---
>  .../landlock/scoped_abstract_unix_test.c      | 204 ++++++++++++++++++
>  1 file changed, 204 insertions(+)
> 
> diff --git a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
> index 232c3b767b8a..21285a7158b6 100644
> --- a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
> +++ b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
> @@ -939,4 +939,208 @@ TEST_F(unix_sock_special_cases, socket_with_different_domain)
>  	    WEXITSTATUS(status) != EXIT_SUCCESS)
>  		_metadata->exit_code = KSFT_FAIL;
>  }
> +
> +static const char path1[] = TMP_DIR "/s1_variant1";
> +static const char path2[] = TMP_DIR "/s2_variant1";
> +
> +/* clang-format off */
> +FIXTURE(pathname_address_sockets) {
> +	struct service_fixture stream_address, dgram_address;
> +};
> +
> +/* clang-format on */

Please minimize the use of these tags (e.g. don't include new lines) and
remove them when they don't change the formatting.

> +	if (WIFSIGNALED(status) || !WIFEXITED(status) ||
> +	    WEXITSTATUS(status) != EXIT_SUCCESS)
> +		_metadata->exit_code = KSFT_FAIL;
> +}

Please always add a newline before TEST_HARNESS_MAIN.
`check-linux.sh all` prints an error.

>  TEST_HARNESS_MAIN
> -- 
> 2.34.1
> 
>