Re: [LTP] [PATCH v2 1/4] Fallback landlock network support

Cyril Hrubis
2024-11-05T13:08:44Z

Hi!
> Landlock network support has been added in the ABI v4, adding features
> for bind() and connect() syscalls. It also defined one more member in
> the landlock_ruleset_attr struct, breaking our LTP fallbacks, used to
> build landlock testing suite. For this reason, we introduce
> tst_landlock_ruleset_attr_abi[14] struct(s) which fallback ABI v1 and v4
> ruleset_attr definitions.
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> configure.ac | 3 ++-
> include/lapi/capability.h | 4 ++++
> include/lapi/landlock.h | 28 ++++++++++++----------
> testcases/kernel/syscalls/landlock/landlock01.c | 15 ++++--------
> testcases/kernel/syscalls/landlock/landlock02.c | 8 +++----
> testcases/kernel/syscalls/landlock/landlock03.c | 6 ++---
> testcases/kernel/syscalls/landlock/landlock04.c | 6 ++---
> testcases/kernel/syscalls/landlock/landlock05.c | 10 ++++----
> testcases/kernel/syscalls/landlock/landlock06.c | 14 ++++-------
> testcases/kernel/syscalls/landlock/landlock07.c | 6 ++---
> .../kernel/syscalls/landlock/landlock_common.h | 12 ++++------
> 11 files changed, 53 insertions(+), 59 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index d327974efa71f263d7f7f5aec9d2c5831d53dd0e..e2e4fd18daa54dbf2034fa9bcc4f2383b53392f4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -34,6 +34,8 @@ m4_ifndef([PKG_CHECK_EXISTS],
> AC_PREFIX_DEFAULT(/opt/ltp)
>
> AC_CHECK_DECLS([IFLA_NET_NS_PID],,,[#include <linux/if_link.h>])
> +AC_CHECK_DECLS([LANDLOCK_RULE_PATH_BENEATH],,,[#include <linux/landlock.h>])
> +AC_CHECK_DECLS([LANDLOCK_RULE_NET_PORT],,,[#include <linux/landlock.h>])
> AC_CHECK_DECLS([MADV_MERGEABLE],,,[#include <sys/mman.h>])
> AC_CHECK_DECLS([NFTA_CHAIN_ID, NFTA_VERDICT_CHAIN_ID],,,[#include <linux/netfilter/nf_tables.h>])
> AC_CHECK_DECLS([PR_CAPBSET_DROP, PR_CAPBSET_READ],,,[#include <sys/prctl.h>])
> @@ -172,7 +174,6 @@ AC_CHECK_MEMBERS([struct utsname.domainname],,,[
> ])
>
> AC_CHECK_TYPES([enum kcmp_type],,,[#include <linux/kcmp.h>])
> -AC_CHECK_TYPES([enum landlock_rule_type],,,[#include <linux/landlock.h>])
> AC_CHECK_TYPES([struct acct_v3],,,[#include <sys/acct.h>])
> AC_CHECK_TYPES([struct af_alg_iv, struct sockaddr_alg],,,[# include <linux/if_alg.h>])
> AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_error,
> diff --git a/include/lapi/capability.h b/include/lapi/capability.h
> index 0f317d6d770e86b399f0fed2de04c1dce6723eae..14d2d3c12c051006875f1f864ec58a88a3870ec0 100644
> --- a/include/lapi/capability.h
> +++ b/include/lapi/capability.h
> @@ -20,6 +20,10 @@
> # endif
> #endif
>
> +#ifndef CAP_NET_BIND_SERVICE
> +# define CAP_NET_BIND_SERVICE 10
> +#endif
> +
> #ifndef CAP_NET_RAW
> # define CAP_NET_RAW 13
> #endif
> diff --git a/include/lapi/landlock.h b/include/lapi/landlock.h
> index 211d171ebecd92d75224369dc7f1d5c5903c9ce7..b3c8c548e661680541cdf6e4a8fb68a3f5029fec 100644
> --- a/include/lapi/landlock.h
> +++ b/include/lapi/landlock.h
> @@ -7,6 +7,7 @@
> #define LAPI_LANDLOCK_H__
>
> #include "config.h"
> +#include <stdint.h>
>
> #ifdef HAVE_LINUX_LANDLOCK_H
> # include <linux/landlock.h>
> @@ -14,13 +15,16 @@
>
> #include "lapi/syscalls.h"
>
> -#ifndef HAVE_STRUCT_LANDLOCK_RULESET_ATTR
> -struct landlock_ruleset_attr
> +struct tst_landlock_ruleset_attr_abi1
> +{
> + uint64_t handled_access_fs;
> +};
> +
> +struct tst_landlock_ruleset_attr_abi4
> {
> uint64_t handled_access_fs;
> uint64_t handled_access_net;
> };
> -#endif
>
> #ifndef HAVE_STRUCT_LANDLOCK_PATH_BENEATH_ATTR
> struct landlock_path_beneath_attr
> @@ -30,12 +34,12 @@ struct landlock_path_beneath_attr
> } __attribute__((packed));
> #endif
>
> -#ifndef HAVE_ENUM_LANDLOCK_RULE_TYPE
> -enum landlock_rule_type
> -{
> - LANDLOCK_RULE_PATH_BENEATH = 1,
> - LANDLOCK_RULE_NET_PORT,
> -};
> +#if !HAVE_DECL_LANDLOCK_RULE_PATH_BENEATH

These are more usually ifndef at least it's more readable.

> +# define LANDLOCK_RULE_PATH_BENEATH 1
> +#endif
> +
> +#if !HAVE_DECL_LANDLOCK_RULE_NET_PORT

Here as well.

> +# define LANDLOCK_RULE_NET_PORT 2
> #endif
>
> #ifndef HAVE_STRUCT_LANDLOCK_NET_PORT_ATTR
> @@ -123,8 +127,7 @@ struct landlock_net_port_attr
> #endif
>
> static inline int safe_landlock_create_ruleset(const char *file, const int lineno,
> - const struct landlock_ruleset_attr *attr,
> - size_t size , uint32_t flags)
> + const void *attr, size_t size , uint32_t flags)
> {
> int rval;
>
> @@ -143,8 +146,7 @@ static inline int safe_landlock_create_ruleset(const char *file, const int linen
> }
>
> static inline int safe_landlock_add_rule(const char *file, const int lineno,
> - int ruleset_fd, enum landlock_rule_type rule_type,
> - const void *rule_attr, uint32_t flags)
> + int ruleset_fd, int rule_type, const void *rule_attr, uint32_t flags)
> {
> int rval;
>
> diff --git a/testcases/kernel/syscalls/landlock/landlock01.c b/testcases/kernel/syscalls/landlock/landlock01.c
> index 083685c64fa6d1c0caab887ee03594ea1426f62f..bd3a37153449b8d75b9671f5c3b3838c701b05ae 100644
> --- a/testcases/kernel/syscalls/landlock/landlock01.c
> +++ b/testcases/kernel/syscalls/landlock/landlock01.c
> @@ -17,14 +17,14 @@
>
> #include "landlock_common.h"
>
> -static struct landlock_ruleset_attr *ruleset_attr;
> -static struct landlock_ruleset_attr *null_attr;
> +static struct tst_landlock_ruleset_attr_abi4 *ruleset_attr;
> +static struct tst_landlock_ruleset_attr_abi4 *null_attr;
> static size_t rule_size;
> static size_t rule_small_size;
> static size_t rule_big_size;
>
> static struct tcase {
> - struct landlock_ruleset_attr **attr;
> + struct tst_landlock_ruleset_attr_abi4 **attr;
> uint64_t access_fs;
> size_t *size;
> uint32_t flags;
> @@ -60,13 +60,8 @@ static void setup(void)
> {
> verify_landlock_is_enabled();
>
> - rule_size = sizeof(struct landlock_ruleset_attr);
> -
> -#ifdef HAVE_STRUCT_LANDLOCK_RULESET_ATTR_HANDLED_ACCESS_NET
> + rule_size = sizeof(struct tst_landlock_ruleset_attr_abi4);
> rule_small_size = rule_size - sizeof(uint64_t) - 1;

I guess that the safest bet here would be:

sizeof(struct tst_landlock_ruleset_attr_abi1) - 1

That is by definition one byte less than the smallest size, this will
also in 99.99% cases evaluate to 7 since structure with single 64 bit
number will not need padding so hardcoding 7 should be safe as well.

Also I guess that we can use the v1 ABI for the whole invalid inputs
tests, all we need here is to pass a size that is valid in most cases,
which is v1 I suppose.

The rest looks fine to me:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

--
Cyril Hrubis
chrubis@suse.cz