linux-kernel - Re: [RFC][PATCH 2/4] bpf: Allow BPF_JIT with CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a7ac9672-06d1-4f6a-b676-01c9868ea39c@csgroup.eu>
Date: Thu, 7 Mar 2024 22:09:31 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Calvin Owens <jcalvinowens@...il.com>, Luis Chamberlain
	<mcgrof@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>, Alexei
 Starovoitov <ast@...nel.org>, Steven Rostedt <rostedt@...dmis.org>, Daniel
 Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Masami
 Hiramatsu <mhiramat@...nel.org>, Naveen N Rao <naveen.n.rao@...ux.ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>, David S Miller
	<davem@...emloft.net>, Thomas Gleixner <tglx@...utronix.de>
CC: "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
	"linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH 2/4] bpf: Allow BPF_JIT with CONFIG_MODULES=n



Le 06/03/2024 à 21:05, Calvin Owens a écrit :
> [Vous ne recevez pas souvent de courriers de jcalvinowens@...il.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> No BPF code has to change, except in struct_ops (for module refs).
> 
> This conflicts with bpf-next because of this (relevant) series:
> 
>      https://lore.kernel.org/all/20240119225005.668602-1-thinker.li@gmail.com/
> 
> If something like this is merged down the road, it can go through
> bpf-next at leisure once the module_alloc change is in: it's a one-way
> dependency.
> 
> Signed-off-by: Calvin Owens <jcalvinowens@...il.com>
> ---
>   kernel/bpf/Kconfig          |  2 +-
>   kernel/bpf/bpf_struct_ops.c | 28 ++++++++++++++++++++++++----
>   2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
> index 6a906ff93006..77df483a8925 100644
> --- a/kernel/bpf/Kconfig
> +++ b/kernel/bpf/Kconfig
> @@ -42,7 +42,7 @@ config BPF_JIT
>          bool "Enable BPF Just In Time compiler"
>          depends on BPF
>          depends on HAVE_CBPF_JIT || HAVE_EBPF_JIT
> -       depends on MODULES
> +       select MODULE_ALLOC
>          help
>            BPF programs are normally handled by a BPF interpreter. This option
>            allows the kernel to generate native code when a program is loaded
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 02068bd0e4d9..fbf08a1bb00c 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -108,11 +108,30 @@ const struct bpf_prog_ops bpf_struct_ops_prog_ops = {
>   #endif
>   };
> 
> +#if IS_ENABLED(CONFIG_MODULES)

Can you avoid ifdefs as much as possible ?

>   static const struct btf_type *module_type;
> 
> +static int bpf_struct_module_type_init(struct btf *btf)
> +{
> +       s32 module_id;

Could be:

	if (!IS_ENABLED(CONFIG_MODULES))
		return 0;

> +
> +       module_id = btf_find_by_name_kind(btf, "module", BTF_KIND_STRUCT);
> +       if (module_id < 0)
> +               return 1;
> +
> +       module_type = btf_type_by_id(btf, module_id);
> +       return 0;
> +}
> +#else
> +static int bpf_struct_module_type_init(struct btf *btf)
> +{
> +       return 0;
> +}
> +#endif
> +
>   void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
>   {
> -       s32 type_id, value_id, module_id;
> +       s32 type_id, value_id;
>          const struct btf_member *member;
>          struct bpf_struct_ops *st_ops;
>          const struct btf_type *t;
> @@ -125,12 +144,10 @@ void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
>   #include "bpf_struct_ops_types.h"
>   #undef BPF_STRUCT_OPS_TYPE
> 
> -       module_id = btf_find_by_name_kind(btf, "module", BTF_KIND_STRUCT);
> -       if (module_id < 0) {
> +       if (bpf_struct_module_type_init(btf)) {
>                  pr_warn("Cannot find struct module in btf_vmlinux\n");
>                  return;
>          }
> -       module_type = btf_type_by_id(btf, module_id);
> 
>          for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
>                  st_ops = bpf_struct_ops[i];
> @@ -433,12 +450,15 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
> 
>                  moff = __btf_member_bit_offset(t, member) / 8;
>                  ptype = btf_type_resolve_ptr(btf_vmlinux, member->type, NULL);
> +
> +#if IS_ENABLED(CONFIG_MODULES)

Can't see anything depending on CONFIG_MODULES here, can you instead do:

		if (IS_ENABLED(CONFIG_MODULES) && ptype == module_type) {

>                  if (ptype == module_type) {
>                          if (*(void **)(udata + moff))
>                                  goto reset_unlock;
>                          *(void **)(kdata + moff) = BPF_MODULE_OWNER;
>                          continue;
>                  }
> +#endif
> 
>                  err = st_ops->init_member(t, member, kdata, udata);
>                  if (err < 0)
> --
> 2.43.0
> 
>