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

Li Wang
2024-11-05T12:43:20Z

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?

--
Regards,
Li Wang