Re: [LTP] [PATCH] landlock08: add UDP bind/connect test variants

[Petr Vorel <pvorel@suse.cz>]

> From: Andrea Cervesato <andrea.cervesato@suse.com>

> Extend the network test to cover the LANDLOCK_ACCESS_NET_BIND_UDP and
> LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP access rights introduced with
> Landlock ABI v10, alongside the existing TCP rights.

Implements: https://github.com/linux-test-project/ltp/issues/1333

Also why not to mention that it's from kernel 7.2? Saves test reviewers to
search for it.

...
> +++ b/testcases/kernel/syscalls/landlock/landlock08.c
> @@ -5,67 +5,103 @@

>  /*\
>   * Verify the landlock support for bind()/connect() syscalls in IPV4 and IPV6
> - * protocols. In particular, check that bind() is assigning the address only on
> - * the TCP port enforced by LANDLOCK_ACCESS_NET_BIND_TCP and check that
> - * connect() is connecting only to a specific TCP port enforced by
> - * LANDLOCK_ACCESS_NET_CONNECT_TCP.
> + * protocols, using both TCP and UDP. In particular, check that bind() is
> + * assigning the address only on the port enforced by
> + * LANDLOCK_ACCESS_NET_BIND_TCP / LANDLOCK_ACCESS_NET_BIND_UDP and check that
> + * connect() is connecting only to a specific port enforced by
> + * LANDLOCK_ACCESS_NET_CONNECT_TCP / LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP.
> + *
> + * TCP rules are available since Landlock ABI v4, while UDP rules are available
> + * since Landlock ABI v10.
Also here I'd add the versions.

>   *
>   * [Algorithm]
>   *
> - * Repeat the following procedure for IPV4 and IPV6:
> + * Repeat the following procedure for {TCP, UDP} x {IPV4, IPV6}:
>   *
>   * - create a socket on PORT1, bind() it and check if it passes
> - * - enforce the current sandbox with LANDLOCK_ACCESS_NET_BIND_TCP on PORT1
> + * - enforce the current sandbox with the BIND access right on PORT1
>   * - create a socket on PORT1, bind() it and check if it passes
>   * - create a socket on PORT2, bind() it and check if it fails
>   *
>   * - create a server listening on PORT1
>   * - create a socket on PORT1, connect() to it and check if it passes
> - * - enforce the current sandbox with LANDLOCK_ACCESS_NET_CONNECT_TCP on PORT1
> + * - enforce the current sandbox with the CONNECT access right on PORT1
>   * - create a socket on PORT1, connect() to it and check if it passes
>   * - create a socket on PORT2, connect() to it and check if it fails
>   */

>  #include "landlock_common.h"

> -static int variants[] = {
> -	AF_INET,
> -	AF_INET6,
> +static struct tcase {
> +	int family;
> +	int type;
> +	uint64_t bind_access;
> +	uint64_t connect_access;
> +	int min_abi;
> +	const char *desc;
> +} variants[] = {
> +	{
> +		AF_INET, SOCK_STREAM,
> +		LANDLOCK_ACCESS_NET_BIND_TCP,
> +		LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +		4, "TCP/IPV4"
very nit: IPv4
> +	},
> +	{
> +		AF_INET6, SOCK_STREAM,
> +		LANDLOCK_ACCESS_NET_BIND_TCP,
> +		LANDLOCK_ACCESS_NET_CONNECT_TCP,
> +		4, "TCP/IPV6"
IPv6
(and below)

> +	},
> +	{
> +		AF_INET, SOCK_DGRAM,
> +		LANDLOCK_ACCESS_NET_BIND_UDP,
> +		LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP,
> +		10, "UDP/IPV4"
> +	},
> +	{
> +		AF_INET6, SOCK_DGRAM,
> +		LANDLOCK_ACCESS_NET_BIND_UDP,
> +		LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP,
> +		10, "UDP/IPV6"
> +	},
>  };
...

> -static void test_connect(const int addr_family, const in_port_t port,
> +static void test_connect(const struct tcase *tc, const in_port_t port,
>  	const int exp_err)
>  {
>  	struct socket_data socket;
>  	struct sockaddr *addr = NULL;

> -	create_socket(&socket, addr_family, port);
> -	getsocket_addr(&socket, addr_family, &addr);
> +	create_socket(&socket, tc->family, port, tc->type);
> +	getsocket_addr(&socket, tc->family, &addr);

>  	if (exp_err) {
>  		TST_EXP_FAIL(
> @@ -102,13 +138,14 @@ static void test_connect(const int addr_family, const in_port_t port,
>  	SAFE_CLOSE(socket.fd);
>  }

> -static int check_ipv6_support(void)
> +static int check_family_support(const struct tcase *tc)
>  {
>  	int fd;

> -	fd = socket(AF_INET6, SOCK_STREAM, 0);
> +	fd = socket(tc->family, tc->type, 0);
>  	if (fd == -1 && errno == EAFNOSUPPORT) {
> -		tst_res(TCONF, "IPv6 not supported in kernel");
> +		tst_res(TCONF, "%s address family not supported in kernel",
> +			tc->family == AF_INET ? "IPV4" : "IPV6");
+1 (but again very nit IPv[46])

...
>  static void setup(void)
>  {
> -	if (verify_landlock_is_enabled() < 4)
> +	landlock_abi = verify_landlock_is_enabled();
> +	if (landlock_abi < 4)
>  		tst_brk(TCONF, "Landlock network is not supported");

>  	addr_port = TST_GET_UNUSED_PORT(AF_INET, SOCK_STREAM);

Also agent complains about checking UDP port separately seems to be valid to me.

The rest LGTM.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp