diff mbox series

[v2,1/4] Fallback landlock network support

Message ID 20241105-landlock_network-v2-1-d58791487919@suse.com
State Superseded
Headers show
Series landlock network coverage support | expand

Commit Message

Andrea Cervesato Nov. 5, 2024, 9:34 a.m. UTC
From: Andrea Cervesato <andrea.cervesato@suse.com>

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(-)

Comments

Li Wang Nov. 5, 2024, 12:31 p.m. UTC | #1
Hi Andrea,

On Tue, Nov 5, 2024 at 5:36 PM Andrea Cervesato <andrea.cervesato@suse.de>
wrote:

> From: Andrea Cervesato <andrea.cervesato@suse.com>
>
> 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
>

Ok, here you achieve two ABI versions for landlock_ruleset_attr,
but with mainline kernel introducing[1] a new field 'scoped' what will
you do next, add one more ABI version 5 if needed? What if the mainline
kernel adds more new fields in the future?

and why _abi1 and _abi4, but not _abi2?

[1] commit 21d52e295 ("landlock: Add abstract UNIX socket scoping")



>  {
>         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
> +# define LANDLOCK_RULE_PATH_BENEATH 1
> +#endif
> +
> +#if !HAVE_DECL_LANDLOCK_RULE_NET_PORT
> +# 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;
> -#else
> -       rule_small_size = rule_size - 1;
> -#endif
>
>         rule_big_size = SAFE_SYSCONF(_SC_PAGESIZE) + 1;
>  }
> @@ -77,7 +72,7 @@ static struct tst_test test = {
>         .setup = setup,
>         .needs_root = 1,
>         .bufs = (struct tst_buffers []) {
> -               {&ruleset_attr, .size = sizeof(struct
> landlock_ruleset_attr)},
> +               {&ruleset_attr, .size = sizeof(struct
> tst_landlock_ruleset_attr_abi4)},
>                 {},
>         },
>         .caps = (struct tst_cap []) {
> diff --git a/testcases/kernel/syscalls/landlock/landlock02.c
> b/testcases/kernel/syscalls/landlock/landlock02.c
> index
> 1a3df69c9cc3eda98e28cfbfd1e12c57e26d0128..8566d407f6d17ab367695125f07d0a80cf4130e5
> 100644
> --- a/testcases/kernel/syscalls/landlock/landlock02.c
> +++ b/testcases/kernel/syscalls/landlock/landlock02.c
> @@ -20,7 +20,7 @@
>
>  #include "landlock_common.h"
>
> -static struct landlock_ruleset_attr *ruleset_attr;
> +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>  static struct landlock_path_beneath_attr *path_beneath_attr;
>  static struct landlock_path_beneath_attr *rule_null;
>  static int ruleset_fd;
> @@ -93,7 +93,7 @@ static void run(unsigned int n)
>         }
>
>         TST_EXP_FAIL(tst_syscall(__NR_landlock_add_rule,
> -                       *tc->fd, tc->rule_type, *tc->attr, tc->flags),
> +               *tc->fd, tc->rule_type, *tc->attr, tc->flags),
>                 tc->exp_errno,
>                 "%s",
>                 tc->msg);
> @@ -106,7 +106,7 @@ static void setup(void)
>         ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE;
>
>         ruleset_fd =
> TST_EXP_FD_SILENT(tst_syscall(__NR_landlock_create_ruleset,
> -               ruleset_attr, sizeof(struct landlock_ruleset_attr), 0));
> +               ruleset_attr, sizeof(struct
> tst_landlock_ruleset_attr_abi1), 0));
>  }
>
>  static void cleanup(void)
> @@ -122,7 +122,7 @@ static struct tst_test test = {
>         .cleanup = cleanup,
>         .needs_root = 1,
>         .bufs = (struct tst_buffers []) {
> -               {&ruleset_attr, .size = sizeof(struct
> landlock_ruleset_attr)},
> +               {&ruleset_attr, .size = sizeof(struct
> tst_landlock_ruleset_attr_abi1)},
>                 {&path_beneath_attr, .size = sizeof(struct
> landlock_path_beneath_attr)},
>                 {},
>         },
> diff --git a/testcases/kernel/syscalls/landlock/landlock03.c
> b/testcases/kernel/syscalls/landlock/landlock03.c
> index
> 224482255b287cef64f579b5707a92a6b5908f8b..150c8cc4e50ee1b41af3c8c01771c51a8715746f
> 100644
> --- a/testcases/kernel/syscalls/landlock/landlock03.c
> +++ b/testcases/kernel/syscalls/landlock/landlock03.c
> @@ -21,7 +21,7 @@
>
>  #define MAX_STACKED_RULESETS 16
>
> -static struct landlock_ruleset_attr *ruleset_attr;
> +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>  static int ruleset_fd = -1;
>  static int ruleset_invalid = -1;
>  static int file_fd = -1;
> @@ -89,7 +89,7 @@ static void setup(void)
>         ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE;
>
>         ruleset_fd =
> TST_EXP_FD_SILENT(tst_syscall(__NR_landlock_create_ruleset,
> -               ruleset_attr, sizeof(struct landlock_ruleset_attr), 0));
> +               ruleset_attr, sizeof(struct
> tst_landlock_ruleset_attr_abi1), 0));
>
>         file_fd = SAFE_OPEN("junk.bin", O_CREAT, 0777);
>  }
> @@ -112,7 +112,7 @@ static struct tst_test test = {
>         .needs_root = 1,
>         .forks_child = 1,
>         .bufs = (struct tst_buffers []) {
> -               {&ruleset_attr, .size = sizeof(struct
> landlock_ruleset_attr)},
> +               {&ruleset_attr, .size = sizeof(struct
> tst_landlock_ruleset_attr_abi1)},
>                 {},
>         },
>         .caps = (struct tst_cap []) {
> diff --git a/testcases/kernel/syscalls/landlock/landlock04.c
> b/testcases/kernel/syscalls/landlock/landlock04.c
> index
> e9dedd45091ecd15cdce2fa7227bbfceb14abb5e..2485591e2196072f81708fc10cebd382e536e2a9
> 100644
> --- a/testcases/kernel/syscalls/landlock/landlock04.c
> +++ b/testcases/kernel/syscalls/landlock/landlock04.c
> @@ -15,7 +15,7 @@
>  #include "landlock_tester.h"
>  #include "tst_safe_stdio.h"
>
> -static struct landlock_ruleset_attr *ruleset_attr;
> +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>  static struct landlock_path_beneath_attr *path_beneath_attr;
>  static int ruleset_fd = -1;
>
> @@ -153,7 +153,7 @@ static void setup(void)
>         ruleset_attr->handled_access_fs = tester_get_all_fs_rules();
>
>         ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
> -               ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
> +               ruleset_attr, sizeof(struct
> tst_landlock_ruleset_attr_abi1), 0);
>
>         /* since our binary is dynamically linked, we need to enable
> dependences
>          * to be read and executed
> @@ -192,7 +192,7 @@ static struct tst_test test = {
>                 NULL,
>         },
>         .bufs = (struct tst_buffers []) {
> -               {&ruleset_attr, .size = sizeof(struct
> landlock_ruleset_attr)},
> +               {&ruleset_attr, .size = sizeof(struct
> tst_landlock_ruleset_attr_abi1)},
>                 {&path_beneath_attr, .size = sizeof(struct
> landlock_path_beneath_attr)},
>                 {},
>         },
> diff --git a/testcases/kernel/syscalls/landlock/landlock05.c
> b/testcases/kernel/syscalls/landlock/landlock05.c
> index
> 703f7d81c336907f360acbe45b42720dc12bac23..3d5048f0ab51b2d7c3eedc82ef80c04935ac5d86
> 100644
> --- a/testcases/kernel/syscalls/landlock/landlock05.c
> +++ b/testcases/kernel/syscalls/landlock/landlock05.c
> @@ -28,7 +28,7 @@
>  #define FILENAME2 DIR2"/file"
>  #define FILENAME3 DIR3"/file"
>
> -static struct landlock_ruleset_attr *ruleset_attr;
> +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>  static struct landlock_path_beneath_attr *path_beneath_attr;
>
>  static void run(void)
> @@ -68,15 +68,15 @@ static void setup(void)
>                 LANDLOCK_ACCESS_FS_REFER;
>
>         ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
> -               ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
> +               ruleset_attr, sizeof(struct
> tst_landlock_ruleset_attr_abi1), 0);
>
> -       apply_landlock_rule(
> +       apply_landlock_fs_rule(
>                 path_beneath_attr,
>                 ruleset_fd,
>                 LANDLOCK_ACCESS_FS_REFER,
>                 DIR1);
>
> -       apply_landlock_rule(
> +       apply_landlock_fs_rule(
>                 path_beneath_attr,
>                 ruleset_fd,
>                 LANDLOCK_ACCESS_FS_REFER,
> @@ -93,7 +93,7 @@ static struct tst_test test = {
>         .needs_root = 1,
>         .forks_child = 1,
>         .bufs = (struct tst_buffers []) {
> -               {&ruleset_attr, .size = sizeof(struct
> landlock_ruleset_attr)},
> +               {&ruleset_attr, .size = sizeof(struct
> tst_landlock_ruleset_attr_abi1)},
>                 {&path_beneath_attr, .size = sizeof(struct
> landlock_path_beneath_attr)},
>                 {},
>         },
> diff --git a/testcases/kernel/syscalls/landlock/landlock06.c
> b/testcases/kernel/syscalls/landlock/landlock06.c
> index
> 1a6e59241557db23b23beabf9863a9c51353757a..74237d11657054985f06431467fbb161ded0c1b6
> 100644
> --- a/testcases/kernel/syscalls/landlock/landlock06.c
> +++ b/testcases/kernel/syscalls/landlock/landlock06.c
> @@ -18,7 +18,7 @@
>  #define MNTPOINT "sandbox"
>  #define FILENAME MNTPOINT"/fifo"
>
> -static struct landlock_ruleset_attr *ruleset_attr;
> +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>  static struct landlock_path_beneath_attr *path_beneath_attr;
>  static int file_fd = -1;
>  static int dev_fd = -1;
> @@ -42,8 +42,6 @@ static void run(void)
>
>  static void setup(void)
>  {
> -       int ruleset_fd;
> -
>         if (verify_landlock_is_enabled() < 5)
>                 tst_brk(TCONF, "LANDLOCK_ACCESS_FS_IOCTL_DEV is not
> supported");
>
> @@ -56,17 +54,13 @@ static void setup(void)
>
>         ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV;
>
> -       ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
> -               ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
> -
> -       apply_landlock_layer(
> +       apply_landlock_fs_layer(
>                 ruleset_attr,
> +               sizeof(struct tst_landlock_ruleset_attr_abi1),
>                 path_beneath_attr,
>                 MNTPOINT,
>                 LANDLOCK_ACCESS_FS_IOCTL_DEV
>         );
> -
> -       SAFE_CLOSE(ruleset_fd);
>  }
>
>  static void cleanup(void)
> @@ -85,7 +79,7 @@ static struct tst_test test = {
>         .needs_root = 1,
>         .forks_child = 1,
>         .bufs = (struct tst_buffers []) {
> -               {&ruleset_attr, .size = sizeof(struct
> landlock_ruleset_attr)},
> +               {&ruleset_attr, .size = sizeof(struct
> tst_landlock_ruleset_attr_abi1)},
>                 {&path_beneath_attr, .size = sizeof(struct
> landlock_path_beneath_attr)},
>                 {},
>         },
> diff --git a/testcases/kernel/syscalls/landlock/landlock07.c
> b/testcases/kernel/syscalls/landlock/landlock07.c
> index
> 6115ad53876209051952873679eb96014b4dd805..8ee614856312d55e573e18f88a6690b50497ee8b
> 100644
> --- a/testcases/kernel/syscalls/landlock/landlock07.c
> +++ b/testcases/kernel/syscalls/landlock/landlock07.c
> @@ -25,7 +25,7 @@
>  #include "lapi/prctl.h"
>  #include "landlock_common.h"
>
> -static struct landlock_ruleset_attr *ruleset_attr;
> +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>  static int ruleset_fd;
>
>  static pid_t spawn_houdini(void)
> @@ -77,7 +77,7 @@ static void setup(void)
>         ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_WRITE_FILE;
>         ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
>                 ruleset_attr,
> -               sizeof(struct landlock_ruleset_attr),
> +               sizeof(struct tst_landlock_ruleset_attr_abi1),
>                 0);
>  }
>
> @@ -93,7 +93,7 @@ static struct tst_test test = {
>         .cleanup = cleanup,
>         .forks_child = 1,
>         .bufs = (struct tst_buffers []) {
> -               {&ruleset_attr, .size = sizeof(struct
> landlock_ruleset_attr)},
> +               {&ruleset_attr, .size = sizeof(struct
> tst_landlock_ruleset_attr_abi1)},
>                 {},
>         },
>         .caps = (struct tst_cap []) {
> diff --git a/testcases/kernel/syscalls/landlock/landlock_common.h
> b/testcases/kernel/syscalls/landlock/landlock_common.h
> index
> da91daeab7b3def7184f611a90273419e4cfa6f2..f3096f4bf15f155f2a00b39c461d0805a76306e5
> 100644
> --- a/testcases/kernel/syscalls/landlock/landlock_common.h
> +++ b/testcases/kernel/syscalls/landlock/landlock_common.h
> @@ -33,7 +33,7 @@ static inline int verify_landlock_is_enabled(void)
>         return abi;
>  }
>
> -static inline void apply_landlock_rule(
> +static inline void apply_landlock_fs_rule(
>         struct landlock_path_beneath_attr *path_beneath_attr,
>         const int ruleset_fd,
>         const int access,
> @@ -57,21 +57,19 @@ static inline void enforce_ruleset(const int
> ruleset_fd)
>         SAFE_LANDLOCK_RESTRICT_SELF(ruleset_fd, 0);
>  }
>
> -static inline void apply_landlock_layer(
> -       struct landlock_ruleset_attr *ruleset_attr,
> +static inline void apply_landlock_fs_layer(
> +       void *ruleset_attr, size_t attr_size,
>         struct landlock_path_beneath_attr *path_beneath_attr,
>         const char *path,
>         const int access)
>  {
>         int ruleset_fd;
>
> -       ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
> -               ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
> +       ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(ruleset_attr, attr_size,
> 0);
>
> -       apply_landlock_rule(path_beneath_attr, ruleset_fd, access, path);
> +       apply_landlock_fs_rule(path_beneath_attr, ruleset_fd, access,
> path);
>         enforce_ruleset(ruleset_fd);
>
>         SAFE_CLOSE(ruleset_fd);
>  }
> -
>  #endif /* LANDLOCK_COMMON_H__ */
>
> --
> 2.43.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
Li Wang Nov. 5, 2024, 12:42 p.m. UTC | #2
On Tue, Nov 5, 2024 at 8:31 PM Li Wang <liwang@redhat.com> wrote:

> Hi Andrea,
>
> On Tue, Nov 5, 2024 at 5:36 PM Andrea Cervesato <andrea.cervesato@suse.de>
> wrote:
>
>> From: Andrea Cervesato <andrea.cervesato@suse.com>
>>
>> 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
>>
>
> Ok, here you achieve two ABI versions for landlock_ruleset_attr,
> but with mainline kernel introducing[1] a new field 'scoped' what will
> you do next, add one more ABI version 5 if needed? What if the mainline
> kernel adds more new fields in the future?
>
> and why _abi1 and _abi4, but not _abi2?
>
> [1] commit 21d52e295 ("landlock: Add abstract UNIX socket scoping")
>

Or, another way is just to define the latest ABI version in lapi/landlock.h,
but only define the tested ABI version in a single test, e.g.
landlock01.c used landlock_ruleset_attr_abi1, so this won't make people
confused when reading the test code, they knows the landlock01 is only
test abi1 and don't need to care about things in 'lapi/landlock.h', WDYT?
Andrea Cervesato Nov. 5, 2024, 12:47 p.m. UTC | #3
Hi Li,

On 11/5/24 13:31, Li Wang wrote:
> Hi Andrea,
>
> On Tue, Nov 5, 2024 at 5:36 PM Andrea Cervesato 
> <andrea.cervesato@suse.de> wrote:
>
>     From: Andrea Cervesato <andrea.cervesato@suse.com>
>
>     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 <http://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 <http://configure.ac> b/configure.ac
>     <http://configure.ac>
>     index
>     d327974efa71f263d7f7f5aec9d2c5831d53dd0e..e2e4fd18daa54dbf2034fa9bcc4f2383b53392f4
>     100644
>     --- a/configure.ac <http://configure.ac>
>     +++ b/configure.ac <http://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
>
>
> Ok, here you achieve two ABI versions for landlock_ruleset_attr,
> but with mainline kernel introducing[1] a new field 'scoped' what will
> you do next, add one more ABI version 5 if needed? What if the mainline
> kernel adds more new fields in the future?
>
> and why _abi1 and _abi4, but not _abi2?
>
> [1] commit 21d52e295 ("landlock: Add abstract UNIX socket scoping")
>
Yes, the idea is that we create an `*_abiN` for specific ABI version 
when it's needed. And then we only use the struct which is needed for 
landlock_create_ruleset.

I guess `scoped` will be in `_abi6`.

Andrea

>      {
>             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
>     +# define LANDLOCK_RULE_PATH_BENEATH 1
>     +#endif
>     +
>     +#if !HAVE_DECL_LANDLOCK_RULE_NET_PORT
>     +# 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;
>     -#else
>     -       rule_small_size = rule_size - 1;
>     -#endif
>
>             rule_big_size = SAFE_SYSCONF(_SC_PAGESIZE) + 1;
>      }
>     @@ -77,7 +72,7 @@ static struct tst_test test = {
>             .setup = setup,
>             .needs_root = 1,
>             .bufs = (struct tst_buffers []) {
>     -               {&ruleset_attr, .size = sizeof(struct
>     landlock_ruleset_attr)},
>     +               {&ruleset_attr, .size = sizeof(struct
>     tst_landlock_ruleset_attr_abi4)},
>                     {},
>             },
>             .caps = (struct tst_cap []) {
>     diff --git a/testcases/kernel/syscalls/landlock/landlock02.c
>     b/testcases/kernel/syscalls/landlock/landlock02.c
>     index
>     1a3df69c9cc3eda98e28cfbfd1e12c57e26d0128..8566d407f6d17ab367695125f07d0a80cf4130e5
>     100644
>     --- a/testcases/kernel/syscalls/landlock/landlock02.c
>     +++ b/testcases/kernel/syscalls/landlock/landlock02.c
>     @@ -20,7 +20,7 @@
>
>      #include "landlock_common.h"
>
>     -static struct landlock_ruleset_attr *ruleset_attr;
>     +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>      static struct landlock_path_beneath_attr *path_beneath_attr;
>      static struct landlock_path_beneath_attr *rule_null;
>      static int ruleset_fd;
>     @@ -93,7 +93,7 @@ static void run(unsigned int n)
>             }
>
>             TST_EXP_FAIL(tst_syscall(__NR_landlock_add_rule,
>     -                       *tc->fd, tc->rule_type, *tc->attr, tc->flags),
>     +               *tc->fd, tc->rule_type, *tc->attr, tc->flags),
>                     tc->exp_errno,
>                     "%s",
>                     tc->msg);
>     @@ -106,7 +106,7 @@ static void setup(void)
>             ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE;
>
>             ruleset_fd =
>     TST_EXP_FD_SILENT(tst_syscall(__NR_landlock_create_ruleset,
>     -               ruleset_attr, sizeof(struct
>     landlock_ruleset_attr), 0));
>     +               ruleset_attr, sizeof(struct
>     tst_landlock_ruleset_attr_abi1), 0));
>      }
>
>      static void cleanup(void)
>     @@ -122,7 +122,7 @@ static struct tst_test test = {
>             .cleanup = cleanup,
>             .needs_root = 1,
>             .bufs = (struct tst_buffers []) {
>     -               {&ruleset_attr, .size = sizeof(struct
>     landlock_ruleset_attr)},
>     +               {&ruleset_attr, .size = sizeof(struct
>     tst_landlock_ruleset_attr_abi1)},
>                     {&path_beneath_attr, .size = sizeof(struct
>     landlock_path_beneath_attr)},
>                     {},
>             },
>     diff --git a/testcases/kernel/syscalls/landlock/landlock03.c
>     b/testcases/kernel/syscalls/landlock/landlock03.c
>     index
>     224482255b287cef64f579b5707a92a6b5908f8b..150c8cc4e50ee1b41af3c8c01771c51a8715746f
>     100644
>     --- a/testcases/kernel/syscalls/landlock/landlock03.c
>     +++ b/testcases/kernel/syscalls/landlock/landlock03.c
>     @@ -21,7 +21,7 @@
>
>      #define MAX_STACKED_RULESETS 16
>
>     -static struct landlock_ruleset_attr *ruleset_attr;
>     +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>      static int ruleset_fd = -1;
>      static int ruleset_invalid = -1;
>      static int file_fd = -1;
>     @@ -89,7 +89,7 @@ static void setup(void)
>             ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE;
>
>             ruleset_fd =
>     TST_EXP_FD_SILENT(tst_syscall(__NR_landlock_create_ruleset,
>     -               ruleset_attr, sizeof(struct
>     landlock_ruleset_attr), 0));
>     +               ruleset_attr, sizeof(struct
>     tst_landlock_ruleset_attr_abi1), 0));
>
>             file_fd = SAFE_OPEN("junk.bin", O_CREAT, 0777);
>      }
>     @@ -112,7 +112,7 @@ static struct tst_test test = {
>             .needs_root = 1,
>             .forks_child = 1,
>             .bufs = (struct tst_buffers []) {
>     -               {&ruleset_attr, .size = sizeof(struct
>     landlock_ruleset_attr)},
>     +               {&ruleset_attr, .size = sizeof(struct
>     tst_landlock_ruleset_attr_abi1)},
>                     {},
>             },
>             .caps = (struct tst_cap []) {
>     diff --git a/testcases/kernel/syscalls/landlock/landlock04.c
>     b/testcases/kernel/syscalls/landlock/landlock04.c
>     index
>     e9dedd45091ecd15cdce2fa7227bbfceb14abb5e..2485591e2196072f81708fc10cebd382e536e2a9
>     100644
>     --- a/testcases/kernel/syscalls/landlock/landlock04.c
>     +++ b/testcases/kernel/syscalls/landlock/landlock04.c
>     @@ -15,7 +15,7 @@
>      #include "landlock_tester.h"
>      #include "tst_safe_stdio.h"
>
>     -static struct landlock_ruleset_attr *ruleset_attr;
>     +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>      static struct landlock_path_beneath_attr *path_beneath_attr;
>      static int ruleset_fd = -1;
>
>     @@ -153,7 +153,7 @@ static void setup(void)
>             ruleset_attr->handled_access_fs = tester_get_all_fs_rules();
>
>             ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
>     -               ruleset_attr, sizeof(struct
>     landlock_ruleset_attr), 0);
>     +               ruleset_attr, sizeof(struct
>     tst_landlock_ruleset_attr_abi1), 0);
>
>             /* since our binary is dynamically linked, we need to
>     enable dependences
>              * to be read and executed
>     @@ -192,7 +192,7 @@ static struct tst_test test = {
>                     NULL,
>             },
>             .bufs = (struct tst_buffers []) {
>     -               {&ruleset_attr, .size = sizeof(struct
>     landlock_ruleset_attr)},
>     +               {&ruleset_attr, .size = sizeof(struct
>     tst_landlock_ruleset_attr_abi1)},
>                     {&path_beneath_attr, .size = sizeof(struct
>     landlock_path_beneath_attr)},
>                     {},
>             },
>     diff --git a/testcases/kernel/syscalls/landlock/landlock05.c
>     b/testcases/kernel/syscalls/landlock/landlock05.c
>     index
>     703f7d81c336907f360acbe45b42720dc12bac23..3d5048f0ab51b2d7c3eedc82ef80c04935ac5d86
>     100644
>     --- a/testcases/kernel/syscalls/landlock/landlock05.c
>     +++ b/testcases/kernel/syscalls/landlock/landlock05.c
>     @@ -28,7 +28,7 @@
>      #define FILENAME2 DIR2"/file"
>      #define FILENAME3 DIR3"/file"
>
>     -static struct landlock_ruleset_attr *ruleset_attr;
>     +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>      static struct landlock_path_beneath_attr *path_beneath_attr;
>
>      static void run(void)
>     @@ -68,15 +68,15 @@ static void setup(void)
>                     LANDLOCK_ACCESS_FS_REFER;
>
>             ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
>     -               ruleset_attr, sizeof(struct
>     landlock_ruleset_attr), 0);
>     +               ruleset_attr, sizeof(struct
>     tst_landlock_ruleset_attr_abi1), 0);
>
>     -       apply_landlock_rule(
>     +       apply_landlock_fs_rule(
>                     path_beneath_attr,
>                     ruleset_fd,
>                     LANDLOCK_ACCESS_FS_REFER,
>                     DIR1);
>
>     -       apply_landlock_rule(
>     +       apply_landlock_fs_rule(
>                     path_beneath_attr,
>                     ruleset_fd,
>                     LANDLOCK_ACCESS_FS_REFER,
>     @@ -93,7 +93,7 @@ static struct tst_test test = {
>             .needs_root = 1,
>             .forks_child = 1,
>             .bufs = (struct tst_buffers []) {
>     -               {&ruleset_attr, .size = sizeof(struct
>     landlock_ruleset_attr)},
>     +               {&ruleset_attr, .size = sizeof(struct
>     tst_landlock_ruleset_attr_abi1)},
>                     {&path_beneath_attr, .size = sizeof(struct
>     landlock_path_beneath_attr)},
>                     {},
>             },
>     diff --git a/testcases/kernel/syscalls/landlock/landlock06.c
>     b/testcases/kernel/syscalls/landlock/landlock06.c
>     index
>     1a6e59241557db23b23beabf9863a9c51353757a..74237d11657054985f06431467fbb161ded0c1b6
>     100644
>     --- a/testcases/kernel/syscalls/landlock/landlock06.c
>     +++ b/testcases/kernel/syscalls/landlock/landlock06.c
>     @@ -18,7 +18,7 @@
>      #define MNTPOINT "sandbox"
>      #define FILENAME MNTPOINT"/fifo"
>
>     -static struct landlock_ruleset_attr *ruleset_attr;
>     +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>      static struct landlock_path_beneath_attr *path_beneath_attr;
>      static int file_fd = -1;
>      static int dev_fd = -1;
>     @@ -42,8 +42,6 @@ static void run(void)
>
>      static void setup(void)
>      {
>     -       int ruleset_fd;
>     -
>             if (verify_landlock_is_enabled() < 5)
>                     tst_brk(TCONF, "LANDLOCK_ACCESS_FS_IOCTL_DEV is
>     not supported");
>
>     @@ -56,17 +54,13 @@ static void setup(void)
>
>             ruleset_attr->handled_access_fs =
>     LANDLOCK_ACCESS_FS_IOCTL_DEV;
>
>     -       ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
>     -               ruleset_attr, sizeof(struct
>     landlock_ruleset_attr), 0);
>     -
>     -       apply_landlock_layer(
>     +       apply_landlock_fs_layer(
>                     ruleset_attr,
>     +               sizeof(struct tst_landlock_ruleset_attr_abi1),
>                     path_beneath_attr,
>                     MNTPOINT,
>                     LANDLOCK_ACCESS_FS_IOCTL_DEV
>             );
>     -
>     -       SAFE_CLOSE(ruleset_fd);
>      }
>
>      static void cleanup(void)
>     @@ -85,7 +79,7 @@ static struct tst_test test = {
>             .needs_root = 1,
>             .forks_child = 1,
>             .bufs = (struct tst_buffers []) {
>     -               {&ruleset_attr, .size = sizeof(struct
>     landlock_ruleset_attr)},
>     +               {&ruleset_attr, .size = sizeof(struct
>     tst_landlock_ruleset_attr_abi1)},
>                     {&path_beneath_attr, .size = sizeof(struct
>     landlock_path_beneath_attr)},
>                     {},
>             },
>     diff --git a/testcases/kernel/syscalls/landlock/landlock07.c
>     b/testcases/kernel/syscalls/landlock/landlock07.c
>     index
>     6115ad53876209051952873679eb96014b4dd805..8ee614856312d55e573e18f88a6690b50497ee8b
>     100644
>     --- a/testcases/kernel/syscalls/landlock/landlock07.c
>     +++ b/testcases/kernel/syscalls/landlock/landlock07.c
>     @@ -25,7 +25,7 @@
>      #include "lapi/prctl.h"
>      #include "landlock_common.h"
>
>     -static struct landlock_ruleset_attr *ruleset_attr;
>     +static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
>      static int ruleset_fd;
>
>      static pid_t spawn_houdini(void)
>     @@ -77,7 +77,7 @@ static void setup(void)
>             ruleset_attr->handled_access_fs =
>     LANDLOCK_ACCESS_FS_WRITE_FILE;
>             ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
>                     ruleset_attr,
>     -               sizeof(struct landlock_ruleset_attr),
>     +               sizeof(struct tst_landlock_ruleset_attr_abi1),
>                     0);
>      }
>
>     @@ -93,7 +93,7 @@ static struct tst_test test = {
>             .cleanup = cleanup,
>             .forks_child = 1,
>             .bufs = (struct tst_buffers []) {
>     -               {&ruleset_attr, .size = sizeof(struct
>     landlock_ruleset_attr)},
>     +               {&ruleset_attr, .size = sizeof(struct
>     tst_landlock_ruleset_attr_abi1)},
>                     {},
>             },
>             .caps = (struct tst_cap []) {
>     diff --git a/testcases/kernel/syscalls/landlock/landlock_common.h
>     b/testcases/kernel/syscalls/landlock/landlock_common.h
>     index
>     da91daeab7b3def7184f611a90273419e4cfa6f2..f3096f4bf15f155f2a00b39c461d0805a76306e5
>     100644
>     --- a/testcases/kernel/syscalls/landlock/landlock_common.h
>     +++ b/testcases/kernel/syscalls/landlock/landlock_common.h
>     @@ -33,7 +33,7 @@ static inline int verify_landlock_is_enabled(void)
>             return abi;
>      }
>
>     -static inline void apply_landlock_rule(
>     +static inline void apply_landlock_fs_rule(
>             struct landlock_path_beneath_attr *path_beneath_attr,
>             const int ruleset_fd,
>             const int access,
>     @@ -57,21 +57,19 @@ static inline void enforce_ruleset(const int
>     ruleset_fd)
>             SAFE_LANDLOCK_RESTRICT_SELF(ruleset_fd, 0);
>      }
>
>     -static inline void apply_landlock_layer(
>     -       struct landlock_ruleset_attr *ruleset_attr,
>     +static inline void apply_landlock_fs_layer(
>     +       void *ruleset_attr, size_t attr_size,
>             struct landlock_path_beneath_attr *path_beneath_attr,
>             const char *path,
>             const int access)
>      {
>             int ruleset_fd;
>
>     -       ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
>     -               ruleset_attr, sizeof(struct
>     landlock_ruleset_attr), 0);
>     +       ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(ruleset_attr,
>     attr_size, 0);
>
>     -       apply_landlock_rule(path_beneath_attr, ruleset_fd, access,
>     path);
>     +       apply_landlock_fs_rule(path_beneath_attr, ruleset_fd,
>     access, path);
>             enforce_ruleset(ruleset_fd);
>
>             SAFE_CLOSE(ruleset_fd);
>      }
>     -
>      #endif /* LANDLOCK_COMMON_H__ */
>
>     -- 
>     2.43.0
>
>
>     -- 
>     Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
>
> -- 
> Regards,
> Li Wang
Andrea Cervesato Nov. 5, 2024, 12:59 p.m. UTC | #4
Hi Li,

On 11/5/24 13:42, Li Wang wrote:
> Or, another way is just to define the latest ABI version in 
> lapi/landlock.h,
> but only define the tested ABI version in a single test, e.g.
> landlock01.c used landlock_ruleset_attr_abi1, so this won't make people
> confused when reading the test code, they knows the landlock01 is only
> test abi1 and don't need to care about things in 'lapi/landlock.h', WDYT?
>
Do you mean like having a complete struct which is passed to a helper, 
taking `landlock_create_ruleset` + ABI version and generating out 
ruleset ID ?

Something like:

struct tst_landlock_ruleset_attr {
     uint64_t handled_access_fs;
     uint64_t handled_access_net;
     uint64_t scoped;
};

int tst_landlock_create_ruleset(int version, struct 
tst_landlock_ruleset_attr *attr) {
     struct landlock_ruleset_attr inner_attr;
     copy_ruleset(attr, &inner_attr);

     return landlock_create_ruleset(&inner_attr, sizeof(struct 
landlock_ruleset_attr), 0);
}

In this way it could work, but we loose guarded buffers which are passed 
to the syscall and might be useful during debugging. In this case we 
should use tst_buffers_alloc(). @Cyril what do you think?

Andrea
Cyril Hrubis Nov. 5, 2024, 1:08 p.m. UTC | #5
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>
Andrea Cervesato Nov. 5, 2024, 1:15 p.m. UTC | #6
Hi Cyril,

On 11/5/24 14:08, Cyril Hrubis wrote:
> 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.
>
We can't use #ifndef because HAVE_DECL_LANDLOCK_RULE_PATH_BENEATH is 
always defined, but it can be 0 or 1 if it's present or not (this is 
what I seen using autoconf). You can check in config.h as well. 
Apparently this is how autoconf handles symbols.
>> +# 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
+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>
Andrea
Cyril Hrubis Nov. 5, 2024, 1:15 p.m. UTC | #7
Hi!
> In this way it could work, but we loose guarded buffers which are passed 
> to the syscall and might be useful during debugging. In this case we 
> should use tst_buffers_alloc(). @Cyril what do you think?

That woudln't work either, since we cannot allocate "half a structure"
can we?

Unfortunatelly I think that having a per API version structures is the
cleanest solution. Because:

- our tests should run everywhere they can, that means that we have to
  use the minimal ABI that is required for the test

- important part of the backward compatibilty testing is that there are
  no accesses past the declared ruleset size, which can be easily done
  by the guarded buffers provided that we allocate exactly the size
  needed

And backward compatibility also means that we have to properly handle
the case when we need newer ABI than currently supported, so the
verify_landlock_enabled() needs a size parameter so that we can check
that the ABI is >= than the minimal ABI the test needs...
Cyril Hrubis Nov. 5, 2024, 3:13 p.m. UTC | #8
Hi!
> And backward compatibility also means that we have to properly handle
> the case when we need newer ABI than currently supported, so the
> verify_landlock_enabled() needs a size parameter so that we can check
> that the ABI is >= than the minimal ABI the test needs...

Looking at verify_landlock_enabled() it does return the ABI already, so
this is covered as well.
Li Wang Nov. 6, 2024, 7:13 a.m. UTC | #9
On Tue, Nov 5, 2024 at 9:15 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > In this way it could work, but we loose guarded buffers which are passed
> > to the syscall and might be useful during debugging. In this case we
> > should use tst_buffers_alloc(). @Cyril what do you think?
>
> That woudln't work either, since we cannot allocate "half a structure"
> can we?
>
> Unfortunatelly I think that having a per API version structures is the
> cleanest solution. Because:
>

Okay, I agree to go this way, unless we find a better solution, and it can
be replaced anytime if we have it in the future.

Andrea, feel free to fix the tiny problems and add my RBT.
Reviewed-by: Li Wang <liwang@redhat.com>



>
> - our tests should run everywhere they can, that means that we have to
>   use the minimal ABI that is required for the test
>
> - important part of the backward compatibilty testing is that there are
>   no accesses past the declared ruleset size, which can be easily done
>   by the guarded buffers provided that we allocate exactly the size
>   needed
>
> And backward compatibility also means that we have to properly handle
> the case when we need newer ABI than currently supported, so the
> verify_landlock_enabled() needs a size parameter so that we can check
> that the ABI is >= than the minimal ABI the test needs...
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
>
diff mbox series

Patch

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
+# define LANDLOCK_RULE_PATH_BENEATH 1
+#endif
+
+#if !HAVE_DECL_LANDLOCK_RULE_NET_PORT
+# 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;
-#else
-	rule_small_size = rule_size - 1;
-#endif
 
 	rule_big_size = SAFE_SYSCONF(_SC_PAGESIZE) + 1;
 }
@@ -77,7 +72,7 @@  static struct tst_test test = {
 	.setup = setup,
 	.needs_root = 1,
 	.bufs = (struct tst_buffers []) {
-		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&ruleset_attr, .size = sizeof(struct tst_landlock_ruleset_attr_abi4)},
 		{},
 	},
 	.caps = (struct tst_cap []) {
diff --git a/testcases/kernel/syscalls/landlock/landlock02.c b/testcases/kernel/syscalls/landlock/landlock02.c
index 1a3df69c9cc3eda98e28cfbfd1e12c57e26d0128..8566d407f6d17ab367695125f07d0a80cf4130e5 100644
--- a/testcases/kernel/syscalls/landlock/landlock02.c
+++ b/testcases/kernel/syscalls/landlock/landlock02.c
@@ -20,7 +20,7 @@ 
 
 #include "landlock_common.h"
 
-static struct landlock_ruleset_attr *ruleset_attr;
+static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
 static struct landlock_path_beneath_attr *path_beneath_attr;
 static struct landlock_path_beneath_attr *rule_null;
 static int ruleset_fd;
@@ -93,7 +93,7 @@  static void run(unsigned int n)
 	}
 
 	TST_EXP_FAIL(tst_syscall(__NR_landlock_add_rule,
-			*tc->fd, tc->rule_type, *tc->attr, tc->flags),
+		*tc->fd, tc->rule_type, *tc->attr, tc->flags),
 		tc->exp_errno,
 		"%s",
 		tc->msg);
@@ -106,7 +106,7 @@  static void setup(void)
 	ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE;
 
 	ruleset_fd = TST_EXP_FD_SILENT(tst_syscall(__NR_landlock_create_ruleset,
-		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0));
+		ruleset_attr, sizeof(struct tst_landlock_ruleset_attr_abi1), 0));
 }
 
 static void cleanup(void)
@@ -122,7 +122,7 @@  static struct tst_test test = {
 	.cleanup = cleanup,
 	.needs_root = 1,
 	.bufs = (struct tst_buffers []) {
-		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&ruleset_attr, .size = sizeof(struct tst_landlock_ruleset_attr_abi1)},
 		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
 		{},
 	},
diff --git a/testcases/kernel/syscalls/landlock/landlock03.c b/testcases/kernel/syscalls/landlock/landlock03.c
index 224482255b287cef64f579b5707a92a6b5908f8b..150c8cc4e50ee1b41af3c8c01771c51a8715746f 100644
--- a/testcases/kernel/syscalls/landlock/landlock03.c
+++ b/testcases/kernel/syscalls/landlock/landlock03.c
@@ -21,7 +21,7 @@ 
 
 #define MAX_STACKED_RULESETS 16
 
-static struct landlock_ruleset_attr *ruleset_attr;
+static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
 static int ruleset_fd = -1;
 static int ruleset_invalid = -1;
 static int file_fd = -1;
@@ -89,7 +89,7 @@  static void setup(void)
 	ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE;
 
 	ruleset_fd = TST_EXP_FD_SILENT(tst_syscall(__NR_landlock_create_ruleset,
-		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0));
+		ruleset_attr, sizeof(struct tst_landlock_ruleset_attr_abi1), 0));
 
 	file_fd = SAFE_OPEN("junk.bin", O_CREAT, 0777);
 }
@@ -112,7 +112,7 @@  static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
 	.bufs = (struct tst_buffers []) {
-		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&ruleset_attr, .size = sizeof(struct tst_landlock_ruleset_attr_abi1)},
 		{},
 	},
 	.caps = (struct tst_cap []) {
diff --git a/testcases/kernel/syscalls/landlock/landlock04.c b/testcases/kernel/syscalls/landlock/landlock04.c
index e9dedd45091ecd15cdce2fa7227bbfceb14abb5e..2485591e2196072f81708fc10cebd382e536e2a9 100644
--- a/testcases/kernel/syscalls/landlock/landlock04.c
+++ b/testcases/kernel/syscalls/landlock/landlock04.c
@@ -15,7 +15,7 @@ 
 #include "landlock_tester.h"
 #include "tst_safe_stdio.h"
 
-static struct landlock_ruleset_attr *ruleset_attr;
+static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
 static struct landlock_path_beneath_attr *path_beneath_attr;
 static int ruleset_fd = -1;
 
@@ -153,7 +153,7 @@  static void setup(void)
 	ruleset_attr->handled_access_fs = tester_get_all_fs_rules();
 
 	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
-		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
+		ruleset_attr, sizeof(struct tst_landlock_ruleset_attr_abi1), 0);
 
 	/* since our binary is dynamically linked, we need to enable dependences
 	 * to be read and executed
@@ -192,7 +192,7 @@  static struct tst_test test = {
 		NULL,
 	},
 	.bufs = (struct tst_buffers []) {
-		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&ruleset_attr, .size = sizeof(struct tst_landlock_ruleset_attr_abi1)},
 		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
 		{},
 	},
diff --git a/testcases/kernel/syscalls/landlock/landlock05.c b/testcases/kernel/syscalls/landlock/landlock05.c
index 703f7d81c336907f360acbe45b42720dc12bac23..3d5048f0ab51b2d7c3eedc82ef80c04935ac5d86 100644
--- a/testcases/kernel/syscalls/landlock/landlock05.c
+++ b/testcases/kernel/syscalls/landlock/landlock05.c
@@ -28,7 +28,7 @@ 
 #define FILENAME2 DIR2"/file"
 #define FILENAME3 DIR3"/file"
 
-static struct landlock_ruleset_attr *ruleset_attr;
+static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
 static struct landlock_path_beneath_attr *path_beneath_attr;
 
 static void run(void)
@@ -68,15 +68,15 @@  static void setup(void)
 		LANDLOCK_ACCESS_FS_REFER;
 
 	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
-		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
+		ruleset_attr, sizeof(struct tst_landlock_ruleset_attr_abi1), 0);
 
-	apply_landlock_rule(
+	apply_landlock_fs_rule(
 		path_beneath_attr,
 		ruleset_fd,
 		LANDLOCK_ACCESS_FS_REFER,
 		DIR1);
 
-	apply_landlock_rule(
+	apply_landlock_fs_rule(
 		path_beneath_attr,
 		ruleset_fd,
 		LANDLOCK_ACCESS_FS_REFER,
@@ -93,7 +93,7 @@  static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
 	.bufs = (struct tst_buffers []) {
-		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&ruleset_attr, .size = sizeof(struct tst_landlock_ruleset_attr_abi1)},
 		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
 		{},
 	},
diff --git a/testcases/kernel/syscalls/landlock/landlock06.c b/testcases/kernel/syscalls/landlock/landlock06.c
index 1a6e59241557db23b23beabf9863a9c51353757a..74237d11657054985f06431467fbb161ded0c1b6 100644
--- a/testcases/kernel/syscalls/landlock/landlock06.c
+++ b/testcases/kernel/syscalls/landlock/landlock06.c
@@ -18,7 +18,7 @@ 
 #define MNTPOINT "sandbox"
 #define FILENAME MNTPOINT"/fifo"
 
-static struct landlock_ruleset_attr *ruleset_attr;
+static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
 static struct landlock_path_beneath_attr *path_beneath_attr;
 static int file_fd = -1;
 static int dev_fd = -1;
@@ -42,8 +42,6 @@  static void run(void)
 
 static void setup(void)
 {
-	int ruleset_fd;
-
 	if (verify_landlock_is_enabled() < 5)
 		tst_brk(TCONF, "LANDLOCK_ACCESS_FS_IOCTL_DEV is not supported");
 
@@ -56,17 +54,13 @@  static void setup(void)
 
 	ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV;
 
-	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
-		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
-
-	apply_landlock_layer(
+	apply_landlock_fs_layer(
 		ruleset_attr,
+		sizeof(struct tst_landlock_ruleset_attr_abi1),
 		path_beneath_attr,
 		MNTPOINT,
 		LANDLOCK_ACCESS_FS_IOCTL_DEV
 	);
-
-	SAFE_CLOSE(ruleset_fd);
 }
 
 static void cleanup(void)
@@ -85,7 +79,7 @@  static struct tst_test test = {
 	.needs_root = 1,
 	.forks_child = 1,
 	.bufs = (struct tst_buffers []) {
-		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&ruleset_attr, .size = sizeof(struct tst_landlock_ruleset_attr_abi1)},
 		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
 		{},
 	},
diff --git a/testcases/kernel/syscalls/landlock/landlock07.c b/testcases/kernel/syscalls/landlock/landlock07.c
index 6115ad53876209051952873679eb96014b4dd805..8ee614856312d55e573e18f88a6690b50497ee8b 100644
--- a/testcases/kernel/syscalls/landlock/landlock07.c
+++ b/testcases/kernel/syscalls/landlock/landlock07.c
@@ -25,7 +25,7 @@ 
 #include "lapi/prctl.h"
 #include "landlock_common.h"
 
-static struct landlock_ruleset_attr *ruleset_attr;
+static struct tst_landlock_ruleset_attr_abi1 *ruleset_attr;
 static int ruleset_fd;
 
 static pid_t spawn_houdini(void)
@@ -77,7 +77,7 @@  static void setup(void)
 	ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_WRITE_FILE;
 	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
 		ruleset_attr,
-		sizeof(struct landlock_ruleset_attr),
+		sizeof(struct tst_landlock_ruleset_attr_abi1),
 		0);
 }
 
@@ -93,7 +93,7 @@  static struct tst_test test = {
 	.cleanup = cleanup,
 	.forks_child = 1,
 	.bufs = (struct tst_buffers []) {
-		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&ruleset_attr, .size = sizeof(struct tst_landlock_ruleset_attr_abi1)},
 		{},
 	},
 	.caps = (struct tst_cap []) {
diff --git a/testcases/kernel/syscalls/landlock/landlock_common.h b/testcases/kernel/syscalls/landlock/landlock_common.h
index da91daeab7b3def7184f611a90273419e4cfa6f2..f3096f4bf15f155f2a00b39c461d0805a76306e5 100644
--- a/testcases/kernel/syscalls/landlock/landlock_common.h
+++ b/testcases/kernel/syscalls/landlock/landlock_common.h
@@ -33,7 +33,7 @@  static inline int verify_landlock_is_enabled(void)
 	return abi;
 }
 
-static inline void apply_landlock_rule(
+static inline void apply_landlock_fs_rule(
 	struct landlock_path_beneath_attr *path_beneath_attr,
 	const int ruleset_fd,
 	const int access,
@@ -57,21 +57,19 @@  static inline void enforce_ruleset(const int ruleset_fd)
 	SAFE_LANDLOCK_RESTRICT_SELF(ruleset_fd, 0);
 }
 
-static inline void apply_landlock_layer(
-	struct landlock_ruleset_attr *ruleset_attr,
+static inline void apply_landlock_fs_layer(
+	void *ruleset_attr, size_t attr_size,
 	struct landlock_path_beneath_attr *path_beneath_attr,
 	const char *path,
 	const int access)
 {
 	int ruleset_fd;
 
-	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
-		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
+	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(ruleset_attr, attr_size, 0);
 
-	apply_landlock_rule(path_beneath_attr, ruleset_fd, access, path);
+	apply_landlock_fs_rule(path_beneath_attr, ruleset_fd, access, path);
 	enforce_ruleset(ruleset_fd);
 
 	SAFE_CLOSE(ruleset_fd);
 }
-
 #endif /* LANDLOCK_COMMON_H__ */