From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761653AbcINSwJ (ORCPT ); Wed, 14 Sep 2016 14:52:09 -0400 Received: from mail-pa0-f65.google.com ([209.85.220.65]:33021 "EHLO mail-pa0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753804AbcINSwF (ORCPT ); Wed, 14 Sep 2016 14:52:05 -0400 Date: Wed, 14 Sep 2016 11:51:58 -0700 From: Alexei Starovoitov To: =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= Cc: linux-kernel@vger.kernel.org, Alexei Starovoitov , Andy Lutomirski , Arnd Bergmann , Casey Schaufler , Daniel Borkmann , Daniel Mack , David Drysdale , "David S . Miller" , Elena Reshetova , "Eric W . Biederman" , James Morris , Kees Cook , Paul Moore , Sargun Dhillon , "Serge E . Hallyn" , Tejun Heo , Will Drewry , kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org, linux-security-module@vger.kernel.org, netdev@vger.kernel.org, cgroups@vger.kernel.org Subject: Re: [RFC v3 03/22] bpf,landlock: Add a new arraymap type to deal with (Landlock) handles Message-ID: <20160914185157.GA53542@ast-mbp.thefacebook.com> References: <20160914072415.26021-1-mic@digikod.net> <20160914072415.26021-4-mic@digikod.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160914072415.26021-4-mic@digikod.net> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Alexei Starovoitov > Cc: Andy Lutomirski > Cc: Daniel Borkmann > Cc: David S. Miller > Cc: Kees Cook > Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com > --- > 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 > #include > > +#ifdef CONFIG_SECURITY_LANDLOCK > +#include /* 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.