On 14/09/2016 20:51, Alexei Starovoitov wrote: > On Wed, Sep 14, 2016 at 09:23:56AM +0200, Mickaël Salaün wrote: >> This new arraymap looks like a set and brings new properties: >> * strong typing of entries: the eBPF functions get the array type of >> elements instead of CONST_PTR_TO_MAP (e.g. >> CONST_PTR_TO_LANDLOCK_HANDLE_FS); >> * force sequential filling (i.e. replace or append-only update), which >> allow quick browsing of all entries. >> >> This strong typing is useful to statically check if the content of a map >> can be passed to an eBPF function. For example, Landlock use it to store >> and manage kernel objects (e.g. struct file) instead of dealing with >> userland raw data. This improve efficiency and ensure that an eBPF >> program can only call functions with the right high-level arguments. >> >> The enum bpf_map_handle_type list low-level types (e.g. >> BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) which are identified when >> updating a map entry (handle). This handle types are used to infer a >> high-level arraymap type which are listed in enum bpf_map_array_type >> (e.g. BPF_MAP_ARRAY_TYPE_LANDLOCK_FS). >> >> For now, this new arraymap is only used by Landlock LSM (cf. next >> commits) but it could be useful for other needs. >> >> Changes since v2: >> * add a RLIMIT_NOFILE-based limit to the maximum number of arraymap >> handle entries (suggested by Andy Lutomirski) >> * remove useless checks >> >> Changes since v1: >> * arraymap of handles replace custom checker groups >> * simpler userland API >> >> Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx> >> Cc: Alexei Starovoitov <ast@xxxxxxxxxx> >> Cc: Andy Lutomirski <luto@xxxxxxxxxxxxxx> >> Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx> >> Cc: David S. Miller <davem@xxxxxxxxxxxxx> >> Cc: Kees Cook <keescook@xxxxxxxxxxxx> >> Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@xxxxxxxxxxxxxx >> --- >> include/linux/bpf.h | 14 ++++ >> include/uapi/linux/bpf.h | 18 +++++ >> kernel/bpf/arraymap.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++ >> kernel/bpf/verifier.c | 12 ++- >> 4 files changed, 246 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h >> index fa9a988400d9..eae4ce4542c1 100644 >> --- a/include/linux/bpf.h >> +++ b/include/linux/bpf.h >> @@ -13,6 +13,10 @@ >> #include <linux/percpu.h> >> #include <linux/err.h> >> >> +#ifdef CONFIG_SECURITY_LANDLOCK >> +#include <linux/fs.h> /* struct file */ >> +#endif /* CONFIG_SECURITY_LANDLOCK */ >> + >> struct perf_event; >> struct bpf_map; >> >> @@ -38,6 +42,7 @@ struct bpf_map_ops { >> struct bpf_map { >> atomic_t refcnt; >> enum bpf_map_type map_type; >> + enum bpf_map_array_type map_array_type; >> u32 key_size; >> u32 value_size; >> u32 max_entries; >> @@ -187,6 +192,9 @@ struct bpf_array { >> */ >> enum bpf_prog_type owner_prog_type; >> bool owner_jited; >> +#ifdef CONFIG_SECURITY_LANDLOCK >> + u32 n_entries; /* number of entries in a handle array */ >> +#endif /* CONFIG_SECURITY_LANDLOCK */ >> union { >> char value[0] __aligned(8); >> void *ptrs[0] __aligned(8); >> @@ -194,6 +202,12 @@ struct bpf_array { >> }; >> }; >> >> +#ifdef CONFIG_SECURITY_LANDLOCK >> +struct map_landlock_handle { >> + u32 type; /* enum bpf_map_handle_type */ >> +}; >> +#endif /* CONFIG_SECURITY_LANDLOCK */ >> + >> #define MAX_TAIL_CALL_CNT 32 >> >> struct bpf_event_entry { >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h >> index 7cd36166f9b7..b68de57f7ab8 100644 >> --- a/include/uapi/linux/bpf.h >> +++ b/include/uapi/linux/bpf.h >> @@ -87,6 +87,15 @@ enum bpf_map_type { >> BPF_MAP_TYPE_PERCPU_ARRAY, >> BPF_MAP_TYPE_STACK_TRACE,P_TYPE_CGROUP_ARRAY >> BPF_MAP_TYPE_CGROUP_ARRAY, >> + BPF_MAP_TYPE_LANDLOCK_ARRAY, >> +}; >> + >> +enum bpf_map_array_type { >> + BPF_MAP_ARRAY_TYPE_UNSPEC, >> +}; >> + >> +enum bpf_map_handle_type { >> + BPF_MAP_HANDLE_TYPE_UNSPEC, >> }; > > missing something. why it has to be special to have it's own > fd array implementation? > Please take a look how BPF_MAP_TYPE_PERF_EVENT_ARRAY, > BPF_MAP_TYPE_CGROUP_ARRAY and BPF_MAP_TYPE_PROG_ARRAY are done. > The all store objects into array map that user space passes via FD. > I think the same model should apply here. The idea is to have multiple way for userland to describe a resource (e.g. an open file descriptor, a path or a glob pattern). The kernel representation could then be a "struct path *" or dedicated types (e.g. custom glob). Another interesting point (that could replace check_map_func_compatibility()) is that BPF_MAP_TYPE_LANDLOCK_ARRAY translate to dedicated (abstract) types (instead of CONST_PTR_TO_MAP) thanks to bpf_reg_type_from_map(). This is useful to abstract userland (map) interface with kernel object(s) dealing with that type. A third point is that BPF_MAP_TYPE_LANDLOCK_ARRAY is a kind of set. It is optimized to quickly walk through all the elements in a sequential way.
Attachment:
signature.asc
Description: OpenPGP digital signature
