* [PATCH bpf-next v5 1/9] kernel: Add kallsyms_on_each_symbol variant for single module
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
@ 2021-12-30 2:36 ` Kumar Kartikeya Dwivedi
2022-01-01 10:03 ` kernel test robot
2021-12-30 2:36 ` [PATCH bpf-next v5 2/9] bpf: Prepare kfunc BTF ID sets when parsing kernel BTF Kumar Kartikeya Dwivedi
` (8 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:36 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Luis Chamberlain, Jessica Yu, Josh Poimboeuf, Jiri Kosina,
Miroslav Benes, Petr Mladek, Joe Lawrence, live-patching,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
The module_kallsyms_on_each_symbol function iterates over symbols of all
modules. To implement BTF ID set processing in each module's BTF parsing
routine, we need a variant that can iterate over a single module's
symbols. To implement this, extract the single module functionality out
of module_kallsyms_on_each_symbol, and rename the old function to
module_kallsyms_on_each_symbol_all.
Then, the new module_kallsyms_on_each_symbol which iterates over a
single module's symbols uses this extracted helper with appropriate
locking.
Next commit will make use of it to implement BTF ID set concatentation
per hook and type.
Also, since we'll be using kallsyms_on_each_symbol for vmlinux BTF
parsing, remove its dependency on CONFIG_LIVEPATCH.
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Miroslav Benes <mbenes@suse.cz>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Joe Lawrence <joe.lawrence@redhat.com>
Cc: live-patching@vger.kernel.org
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/kallsyms.h | 11 ++++++-
include/linux/module.h | 37 +++++++++++++++++++++++-
kernel/kallsyms.c | 4 +--
kernel/livepatch/core.c | 2 +-
kernel/module.c | 62 ++++++++++++++++++++++++++++++----------
5 files changed, 95 insertions(+), 21 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 4176c7eca7b5..89ed3eb2e185 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -65,11 +65,12 @@ static inline void *dereference_symbol_descriptor(void *ptr)
return ptr;
}
+#ifdef CONFIG_KALLSYMS
+
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
void *data);
-#ifdef CONFIG_KALLSYMS
/* Lookup the address for a symbol. Returns 0 if not found. */
unsigned long kallsyms_lookup_name(const char *name);
@@ -98,6 +99,14 @@ extern bool kallsyms_show_value(const struct cred *cred);
#else /* !CONFIG_KALLSYMS */
+static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+ struct module *,
+ unsigned long),
+ void *data)
+{
+ return 0;
+}
+
static inline unsigned long kallsyms_lookup_name(const char *name)
{
return 0;
diff --git a/include/linux/module.h b/include/linux/module.h
index c9f1200b2312..e982aca57883 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -867,8 +867,43 @@ static inline bool module_sig_ok(struct module *module)
}
#endif /* CONFIG_MODULE_SIG */
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+#if defined(CONFIG_MODULES) && defined(CONFIG_KALLSYMS)
+
+#ifdef CONFIG_LIVEPATCH
+
+int module_kallsyms_on_each_symbol_all(int (*fn)(void *, const char *,
+ struct module *,
+ unsigned long),
+ void *data);
+
+#else /* !CONFIG_LIVEPATCH */
+
+static inline int module_kallsyms_on_each_symbol_all(int (*fn)(void *, const char *,
+ struct module *,
+ unsigned long),
+ void *data)
+{
+ return 0;
+}
+
+#endif /* CONFIG_LIVEPATCH */
+
+int module_kallsyms_on_each_symbol(struct module *mod,
+ int (*fn)(void *, const char *,
struct module *, unsigned long),
void *data);
+#else /* !(CONFIG_MODULES && CONFIG_KALLSYMS) */
+
+static inline int module_kallsyms_on_each_symbol(struct module *mod,
+ int (*fn)(void *, const char *,
+ struct module *,
+ unsigned long),
+ void *data)
+{
+ return 0;
+}
+
+#endif /* CONFIG_MODULES && CONFIG_KALLSYMS */
+
#endif /* _LINUX_MODULE_H */
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 3011bc33a5ba..da40f48f071e 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -224,10 +224,9 @@ unsigned long kallsyms_lookup_name(const char *name)
return module_kallsyms_lookup_name(name);
}
-#ifdef CONFIG_LIVEPATCH
/*
* Iterate over all symbols in vmlinux. For symbols from modules use
- * module_kallsyms_on_each_symbol instead.
+ * module_kallsyms_on_each_symbol_all instead.
*/
int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
unsigned long),
@@ -246,7 +245,6 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
}
return 0;
}
-#endif /* CONFIG_LIVEPATCH */
static unsigned long get_symbol_pos(unsigned long addr,
unsigned long *symbolsize,
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 335d988bd811..3756071658fd 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -165,7 +165,7 @@ static int klp_find_object_symbol(const char *objname, const char *name,
};
if (objname)
- module_kallsyms_on_each_symbol(klp_find_callback, &args);
+ module_kallsyms_on_each_symbol_all(klp_find_callback, &args);
else
kallsyms_on_each_symbol(klp_find_callback, &args);
diff --git a/kernel/module.c b/kernel/module.c
index 84a9141a5e15..88a24dd8f4bd 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4473,13 +4473,54 @@ unsigned long module_kallsyms_lookup_name(const char *name)
return ret;
}
-#ifdef CONFIG_LIVEPATCH
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+static int __module_kallsyms_on_each_symbol(struct mod_kallsyms *kallsyms,
+ struct module *mod,
+ int (*fn)(void *, const char *,
+ struct module *, unsigned long),
+ void *data)
+{
+ unsigned long i;
+ int ret = 0;
+
+ for (i = 0; i < kallsyms->num_symtab; i++) {
+ const Elf_Sym *sym = &kallsyms->symtab[i];
+
+ if (sym->st_shndx == SHN_UNDEF)
+ continue;
+
+ ret = fn(data, kallsyms_symbol_name(kallsyms, i),
+ mod, kallsyms_symbol_value(sym));
+ if (ret != 0)
+ break;
+ }
+
+ return ret;
+}
+
+int module_kallsyms_on_each_symbol(struct module *mod,
+ int (*fn)(void *, const char *,
struct module *, unsigned long),
void *data)
+{
+ struct mod_kallsyms *kallsyms;
+ int ret = 0;
+
+ mutex_lock(&module_mutex);
+ /* We hold module_mutex: no need for rcu_dereference_sched */
+ kallsyms = mod->kallsyms;
+ if (mod->state != MODULE_STATE_UNFORMED)
+ ret = __module_kallsyms_on_each_symbol(kallsyms, mod, fn, data);
+ mutex_unlock(&module_mutex);
+
+ return ret;
+}
+
+#ifdef CONFIG_LIVEPATCH
+int module_kallsyms_on_each_symbol_all(int (*fn)(void *, const char *,
+ struct module *, unsigned long),
+ void *data)
{
struct module *mod;
- unsigned int i;
int ret = 0;
mutex_lock(&module_mutex);
@@ -4489,19 +4530,10 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- for (i = 0; i < kallsyms->num_symtab; i++) {
- const Elf_Sym *sym = &kallsyms->symtab[i];
-
- if (sym->st_shndx == SHN_UNDEF)
- continue;
-
- ret = fn(data, kallsyms_symbol_name(kallsyms, i),
- mod, kallsyms_symbol_value(sym));
- if (ret != 0)
- goto out;
- }
+ ret = __module_kallsyms_on_each_symbol(kallsyms, mod, fn, data);
+ if (ret != 0)
+ break;
}
-out:
mutex_unlock(&module_mutex);
return ret;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH bpf-next v5 1/9] kernel: Add kallsyms_on_each_symbol variant for single module
2021-12-30 2:36 ` [PATCH bpf-next v5 1/9] kernel: Add kallsyms_on_each_symbol variant for single module Kumar Kartikeya Dwivedi
@ 2022-01-01 10:03 ` kernel test robot
0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2022-01-01 10:03 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi, bpf, netdev, netfilter-devel
Cc: kbuild-all, Luis Chamberlain, Jessica Yu, Josh Poimboeuf,
Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence
Hi Kumar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/Kumar-Kartikeya-Dwivedi/Introduce-unstable-CT-lookup-helpers/20211230-103958
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: ia64-randconfig-s031-20211230 (https://download.01.org/0day-ci/archive/20220101/202201011858.vOPCCeDV-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/25d6b438335605e4e002f7afde50a3eaf17a0b6c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kumar-Kartikeya-Dwivedi/Introduce-unstable-CT-lookup-helpers/20211230-103958
git checkout 25d6b438335605e4e002f7afde50a3eaf17a0b6c
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
kernel/module.c:2762:23: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct mod_kallsyms [noderef] __rcu *kallsyms @@ got void * @@
kernel/module.c:2762:23: sparse: expected struct mod_kallsyms [noderef] __rcu *kallsyms
kernel/module.c:2762:23: sparse: got void *
>> kernel/module.c:4510:18: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct mod_kallsyms *kallsyms @@ got struct mod_kallsyms [noderef] __rcu *kallsyms @@
kernel/module.c:4510:18: sparse: expected struct mod_kallsyms *kallsyms
kernel/module.c:4510:18: sparse: got struct mod_kallsyms [noderef] __rcu *kallsyms
kernel/module.c:2764:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2765:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2767:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2768:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2777:18: sparse: sparse: dereference of noderef expression
kernel/module.c:2778:35: sparse: sparse: dereference of noderef expression
kernel/module.c:2779:20: sparse: sparse: dereference of noderef expression
kernel/module.c:2784:32: sparse: sparse: dereference of noderef expression
kernel/module.c:2787:45: sparse: sparse: dereference of noderef expression
vim +4510 kernel/module.c
4499
4500 int module_kallsyms_on_each_symbol(struct module *mod,
4501 int (*fn)(void *, const char *,
4502 struct module *, unsigned long),
4503 void *data)
4504 {
4505 struct mod_kallsyms *kallsyms;
4506 int ret = 0;
4507
4508 mutex_lock(&module_mutex);
4509 /* We hold module_mutex: no need for rcu_dereference_sched */
> 4510 kallsyms = mod->kallsyms;
4511 if (mod->state != MODULE_STATE_UNFORMED)
4512 ret = __module_kallsyms_on_each_symbol(kallsyms, mod, fn, data);
4513 mutex_unlock(&module_mutex);
4514
4515 return ret;
4516 }
4517
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH bpf-next v5 1/9] kernel: Add kallsyms_on_each_symbol variant for single module
@ 2022-01-01 10:03 ` kernel test robot
0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2022-01-01 10:03 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]
Hi Kumar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/Kumar-Kartikeya-Dwivedi/Introduce-unstable-CT-lookup-helpers/20211230-103958
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: ia64-randconfig-s031-20211230 (https://download.01.org/0day-ci/archive/20220101/202201011858.vOPCCeDV-lkp(a)intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/25d6b438335605e4e002f7afde50a3eaf17a0b6c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kumar-Kartikeya-Dwivedi/Introduce-unstable-CT-lookup-helpers/20211230-103958
git checkout 25d6b438335605e4e002f7afde50a3eaf17a0b6c
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
kernel/module.c:2762:23: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct mod_kallsyms [noderef] __rcu *kallsyms @@ got void * @@
kernel/module.c:2762:23: sparse: expected struct mod_kallsyms [noderef] __rcu *kallsyms
kernel/module.c:2762:23: sparse: got void *
>> kernel/module.c:4510:18: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct mod_kallsyms *kallsyms @@ got struct mod_kallsyms [noderef] __rcu *kallsyms @@
kernel/module.c:4510:18: sparse: expected struct mod_kallsyms *kallsyms
kernel/module.c:4510:18: sparse: got struct mod_kallsyms [noderef] __rcu *kallsyms
kernel/module.c:2764:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2765:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2767:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2768:12: sparse: sparse: dereference of noderef expression
kernel/module.c:2777:18: sparse: sparse: dereference of noderef expression
kernel/module.c:2778:35: sparse: sparse: dereference of noderef expression
kernel/module.c:2779:20: sparse: sparse: dereference of noderef expression
kernel/module.c:2784:32: sparse: sparse: dereference of noderef expression
kernel/module.c:2787:45: sparse: sparse: dereference of noderef expression
vim +4510 kernel/module.c
4499
4500 int module_kallsyms_on_each_symbol(struct module *mod,
4501 int (*fn)(void *, const char *,
4502 struct module *, unsigned long),
4503 void *data)
4504 {
4505 struct mod_kallsyms *kallsyms;
4506 int ret = 0;
4507
4508 mutex_lock(&module_mutex);
4509 /* We hold module_mutex: no need for rcu_dereference_sched */
> 4510 kallsyms = mod->kallsyms;
4511 if (mod->state != MODULE_STATE_UNFORMED)
4512 ret = __module_kallsyms_on_each_symbol(kallsyms, mod, fn, data);
4513 mutex_unlock(&module_mutex);
4514
4515 return ret;
4516 }
4517
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH bpf-next v5 2/9] bpf: Prepare kfunc BTF ID sets when parsing kernel BTF
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
2021-12-30 2:36 ` [PATCH bpf-next v5 1/9] kernel: Add kallsyms_on_each_symbol variant for single module Kumar Kartikeya Dwivedi
@ 2021-12-30 2:36 ` Kumar Kartikeya Dwivedi
2021-12-31 2:23 ` Alexei Starovoitov
2021-12-30 2:36 ` [PATCH bpf-next v5 3/9] bpf: Remove check_kfunc_call callback and old kfunc BTF ID API Kumar Kartikeya Dwivedi
` (7 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:36 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
This commit prepares the BTF parsing functions for vmlinux and module
BTFs to find all kfunc BTF ID sets from the vmlinux and module symbols
and concatentate all sets into single unified set which is sorted and
keyed by the 'hook' it is meant for, and 'type' of set.
The 'hook' is one of the many program types, e.g. XDP and TC/SCHED_CLS,
STRUCT_OPS, and 'types' are check (allowed or not), acquire, release,
and ret_null (with PTR_TO_BTF_ID_OR_NULL return type).
A maximum of BTF_KFUNC_SET_MAX_CNT (32) kfunc BTF IDs are permitted in a
set of certain hook and type. They are also allocated on demand, and
otherwise set as NULL.
A new btf_kfunc_id_set_contains function is exposed for use in verifier,
this new method is faster than the existing list searching method, and
is also automatic. It also lets other code not care whether the set is
unallocated or not.
Next commit will update the kernel users to make use of this
infrastructure.
Finally, add __maybe_unused annotation for BTF ID macros for the
!CONFIG_DEBUG_INFO_BTF case , so that they don't produce warnings during
build time.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
fixup maybe_unused
---
include/linux/btf.h | 25 ++++
include/linux/btf_ids.h | 20 ++-
kernel/bpf/btf.c | 275 +++++++++++++++++++++++++++++++++++++++-
3 files changed, 312 insertions(+), 8 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 0c74348cbc9d..48ac2dc437a2 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -300,6 +300,21 @@ static inline const struct btf_var_secinfo *btf_type_var_secinfo(
return (const struct btf_var_secinfo *)(t + 1);
}
+enum btf_kfunc_hook {
+ BTF_KFUNC_HOOK_XDP,
+ BTF_KFUNC_HOOK_TC,
+ BTF_KFUNC_HOOK_STRUCT_OPS,
+ _BTF_KFUNC_HOOK_MAX,
+};
+
+enum btf_kfunc_type {
+ BTF_KFUNC_TYPE_CHECK,
+ BTF_KFUNC_TYPE_ACQUIRE,
+ BTF_KFUNC_TYPE_RELEASE,
+ BTF_KFUNC_TYPE_RET_NULL,
+ _BTF_KFUNC_TYPE_MAX,
+};
+
#ifdef CONFIG_BPF_SYSCALL
struct bpf_prog;
@@ -307,6 +322,9 @@ const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
const char *btf_name_by_offset(const struct btf *btf, u32 offset);
struct btf *btf_parse_vmlinux(void);
struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog);
+bool btf_kfunc_id_set_contains(const struct btf *btf,
+ enum bpf_prog_type prog_type,
+ enum btf_kfunc_type type, u32 kfunc_btf_id);
#else
static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
u32 type_id)
@@ -318,6 +336,13 @@ static inline const char *btf_name_by_offset(const struct btf *btf,
{
return NULL;
}
+static inline bool btf_kfunc_id_set_contains(const struct btf *btf,
+ enum bpf_prog_type prog_type,
+ enum btf_kfunc_type type,
+ u32 kfunc_btf_id)
+{
+ return false;
+}
#endif
struct kfunc_btf_id_set {
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index 919c0fde1c51..835fbf626ef1 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -11,6 +11,7 @@ struct btf_id_set {
#ifdef CONFIG_DEBUG_INFO_BTF
#include <linux/compiler.h> /* for __PASTE */
+#include <linux/compiler_attributes.h> /* for __maybe_unused */
/*
* Following macros help to define lists of BTF IDs placed
@@ -144,17 +145,24 @@ asm( \
".popsection; \n"); \
extern struct btf_id_set name;
+#define BTF_KFUNC_SET_START(hook, type, name) \
+ BTF_SET_START(btf_kfunc_set_##hook##_##type##_##name)
+#define BTF_KFUNC_SET_END(hook, type, name) \
+ BTF_SET_END(btf_kfunc_set_##hook##_##type##_##name)
+
#else
-#define BTF_ID_LIST(name) static u32 name[5];
+#define BTF_ID_LIST(name) static u32 __maybe_unused name[5];
#define BTF_ID(prefix, name)
#define BTF_ID_UNUSED
-#define BTF_ID_LIST_GLOBAL(name, n) u32 name[n];
-#define BTF_ID_LIST_SINGLE(name, prefix, typename) static u32 name[1];
-#define BTF_ID_LIST_GLOBAL_SINGLE(name, prefix, typename) u32 name[1];
-#define BTF_SET_START(name) static struct btf_id_set name = { 0 };
-#define BTF_SET_START_GLOBAL(name) static struct btf_id_set name = { 0 };
+#define BTF_ID_LIST_GLOBAL(name, n) u32 __maybe_unused name[n];
+#define BTF_ID_LIST_SINGLE(name, prefix, typename) static u32 __maybe_unused name[1];
+#define BTF_ID_LIST_GLOBAL_SINGLE(name, prefix, typename) u32 __maybe_unused name[1];
+#define BTF_SET_START(name) static struct btf_id_set __maybe_unused name = { 0 };
+#define BTF_SET_START_GLOBAL(name) static struct btf_id_set __maybe_unused name = { 0 };
#define BTF_SET_END(name)
+#define BTF_KFUNC_SET_START(hook, type, name) BTF_SET_START(name)
+#define BTF_KFUNC_SET_END(hook, type, name) BTF_SET_END(name)
#endif /* CONFIG_DEBUG_INFO_BTF */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 33bb8ae4a804..c03c7b5a417c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1,6 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0 */
/* Copyright (c) 2018 Facebook */
+#include <linux/kallsyms.h>
+#include <linux/module.h>
#include <uapi/linux/btf.h>
#include <uapi/linux/bpf.h>
#include <uapi/linux/bpf_perf_event.h>
@@ -198,6 +200,8 @@
DEFINE_IDR(btf_idr);
DEFINE_SPINLOCK(btf_idr_lock);
+struct btf_kfunc_set_tab;
+
struct btf {
void *data;
struct btf_type **types;
@@ -212,6 +216,7 @@ struct btf {
refcount_t refcnt;
u32 id;
struct rcu_head rcu;
+ struct btf_kfunc_set_tab *kfunc_set_tab;
/* split BTF support */
struct btf *base_btf;
@@ -221,6 +226,31 @@ struct btf {
bool kernel_btf;
};
+#define BTF_KFUNC_SET_PREFIX "btf_kfunc_set_"
+
+BTF_ID_LIST_SINGLE(btf_id_set_id, struct, btf_id_set)
+
+static const char *kfunc_hook_str[_BTF_KFUNC_HOOK_MAX] = {
+ [BTF_KFUNC_HOOK_XDP] = "xdp_",
+ [BTF_KFUNC_HOOK_TC] = "tc_",
+ [BTF_KFUNC_HOOK_STRUCT_OPS] = "struct_ops_",
+};
+
+static const char *kfunc_type_str[_BTF_KFUNC_TYPE_MAX] = {
+ [BTF_KFUNC_TYPE_CHECK] = "check_",
+ [BTF_KFUNC_TYPE_ACQUIRE] = "acquire_",
+ [BTF_KFUNC_TYPE_RELEASE] = "release_",
+ [BTF_KFUNC_TYPE_RET_NULL] = "ret_null_",
+};
+
+enum {
+ BTF_KFUNC_SET_MAX_CNT = 32,
+};
+
+struct btf_kfunc_set_tab {
+ struct btf_id_set *sets[_BTF_KFUNC_HOOK_MAX][_BTF_KFUNC_TYPE_MAX];
+};
+
enum verifier_phase {
CHECK_META,
CHECK_TYPE,
@@ -1531,8 +1561,21 @@ static void btf_free_id(struct btf *btf)
spin_unlock_irqrestore(&btf_idr_lock, flags);
}
+static void btf_free_kfunc_set_tab(struct btf_kfunc_set_tab *tab)
+{
+ int hook, type;
+
+ if (!tab)
+ return;
+ for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++) {
+ for (type = 0; type < ARRAY_SIZE(tab->sets[0]); type++)
+ kfree(tab->sets[hook][type]);
+ }
+}
+
static void btf_free(struct btf *btf)
{
+ btf_free_kfunc_set_tab(btf->kfunc_set_tab);
kvfree(btf->types);
kvfree(btf->resolved_sizes);
kvfree(btf->resolved_ids);
@@ -4675,6 +4718,223 @@ static int btf_translate_to_vmlinux(struct bpf_verifier_log *log,
BTF_ID_LIST(bpf_ctx_convert_btf_id)
BTF_ID(struct, bpf_ctx_convert)
+struct btf_parse_kfunc_data {
+ struct btf *btf;
+ struct bpf_verifier_log *log;
+};
+
+static int btf_populate_kfunc_sets(struct btf *btf,
+ struct bpf_verifier_log *log,
+ enum btf_kfunc_hook hook,
+ enum btf_kfunc_type type,
+ struct btf_id_set *add_set)
+{
+ struct btf_id_set *set, *tmp_set;
+ struct btf_kfunc_set_tab *tab;
+ u32 set_cnt;
+ int ret;
+
+ if (WARN_ON_ONCE(hook >= _BTF_KFUNC_HOOK_MAX || type >= _BTF_KFUNC_TYPE_MAX))
+ return -EINVAL;
+ if (!add_set->cnt)
+ return 0;
+
+ tab = btf->kfunc_set_tab;
+ if (!tab) {
+ tab = kzalloc(sizeof(*tab), GFP_KERNEL | __GFP_NOWARN);
+ if (!tab)
+ return -ENOMEM;
+ btf->kfunc_set_tab = tab;
+ }
+
+ set = tab->sets[hook][type];
+ set_cnt = set ? set->cnt : 0;
+
+ if (set_cnt > U32_MAX - add_set->cnt) {
+ ret = -EOVERFLOW;
+ goto end;
+ }
+
+ if (set_cnt + add_set->cnt > BTF_KFUNC_SET_MAX_CNT) {
+ bpf_log(log, "max kfunc (%d) for hook '%s' type '%s' exceeded\n",
+ BTF_KFUNC_SET_MAX_CNT, kfunc_hook_str[hook], kfunc_type_str[type]);
+ ret = -E2BIG;
+ goto end;
+ }
+
+ /* Grow set */
+ tmp_set = krealloc(set, offsetof(struct btf_id_set, ids[set_cnt + add_set->cnt]),
+ GFP_KERNEL | __GFP_NOWARN);
+ if (!tmp_set) {
+ ret = -ENOMEM;
+ goto end;
+ }
+
+ /* For newly allocated set, initialize set->cnt to 0 */
+ if (!set)
+ tmp_set->cnt = 0;
+
+ tab->sets[hook][type] = tmp_set;
+ set = tmp_set;
+
+ /* Concatenate the two sets */
+ memcpy(set->ids + set->cnt, add_set->ids, add_set->cnt * sizeof(set->ids[0]));
+ set->cnt += add_set->cnt;
+
+ return 0;
+end:
+ btf_free_kfunc_set_tab(tab);
+ btf->kfunc_set_tab = NULL;
+ return ret;
+}
+
+static int btf_kfunc_ids_cmp(const void *a, const void *b)
+{
+ const u32 *id1 = a;
+ const u32 *id2 = b;
+
+ if (*id1 < *id2)
+ return -1;
+ if (*id1 > *id2)
+ return 1;
+ return 0;
+}
+
+static int btf_parse_kfunc_sets_cb(void *data, const char *symbol_name,
+ struct module *mod,
+ unsigned long symbol_value)
+{
+ int pfx_size = sizeof(BTF_KFUNC_SET_PREFIX) - 1;
+ struct btf_id_set *set = (void *)symbol_value;
+ struct btf_parse_kfunc_data *bdata = data;
+ const char *orig_name = symbol_name;
+ int i, hook, type;
+
+ BUILD_BUG_ON(ARRAY_SIZE(kfunc_hook_str) != _BTF_KFUNC_HOOK_MAX);
+ BUILD_BUG_ON(ARRAY_SIZE(kfunc_type_str) != _BTF_KFUNC_TYPE_MAX);
+
+ if (strncmp(symbol_name, BTF_KFUNC_SET_PREFIX, pfx_size))
+ return 0;
+
+ /* Identify hook */
+ symbol_name += pfx_size;
+ if (!*symbol_name) {
+ bpf_log(bdata->log, "incomplete kfunc btf_id_set specification: %s\n", orig_name);
+ return -EINVAL;
+ }
+ for (i = 0; i < ARRAY_SIZE(kfunc_hook_str); i++) {
+ pfx_size = strlen(kfunc_hook_str[i]);
+ if (strncmp(symbol_name, kfunc_hook_str[i], pfx_size))
+ continue;
+ break;
+ }
+ if (i == ARRAY_SIZE(kfunc_hook_str)) {
+ bpf_log(bdata->log, "invalid hook '%s' for kfunc_btf_id_set %s\n", symbol_name,
+ orig_name);
+ return -EINVAL;
+ }
+ hook = i;
+
+ /* Identify type */
+ symbol_name += pfx_size;
+ if (!*symbol_name) {
+ bpf_log(bdata->log, "incomplete kfunc btf_id_set specification: %s\n", orig_name);
+ return -EINVAL;
+ }
+ for (i = 0; i < ARRAY_SIZE(kfunc_type_str); i++) {
+ pfx_size = strlen(kfunc_type_str[i]);
+ if (strncmp(symbol_name, kfunc_type_str[i], pfx_size))
+ continue;
+ break;
+ }
+ if (i == ARRAY_SIZE(kfunc_type_str)) {
+ bpf_log(bdata->log, "invalid type '%s' for kfunc_btf_id_set %s\n", symbol_name,
+ orig_name);
+ return -EINVAL;
+ }
+ type = i;
+
+ return btf_populate_kfunc_sets(bdata->btf, bdata->log, hook, type, set);
+}
+
+static int btf_parse_kfunc_sets(struct btf *btf, struct module *mod,
+ struct bpf_verifier_log *log)
+{
+ struct btf_parse_kfunc_data data = { .btf = btf, .log = log, };
+ struct btf_kfunc_set_tab *tab;
+ int hook, type, ret;
+
+ if (!btf_is_kernel(btf))
+ return -EINVAL;
+ if (WARN_ON_ONCE(btf_is_module(btf) && !mod)) {
+ bpf_log(log, "btf internal error: no module for module BTF %s\n", btf->name);
+ return -EFAULT;
+ }
+ if (mod)
+ ret = module_kallsyms_on_each_symbol(mod, btf_parse_kfunc_sets_cb, &data);
+ else
+ ret = kallsyms_on_each_symbol(btf_parse_kfunc_sets_cb, &data);
+
+ tab = btf->kfunc_set_tab;
+ if (!ret && tab) {
+ /* Sort all populated sets */
+ for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++) {
+ for (type = 0; type < ARRAY_SIZE(tab->sets[0]); type++) {
+ struct btf_id_set *set = tab->sets[hook][type];
+
+ /* Not all sets may be populated */
+ if (!set)
+ continue;
+ sort(set->ids, set->cnt, sizeof(set->ids[0]), btf_kfunc_ids_cmp,
+ NULL);
+ }
+ }
+ }
+
+ return 0;
+}
+
+static bool __btf_kfunc_id_set_contains(const struct btf *btf,
+ enum btf_kfunc_hook hook,
+ enum btf_kfunc_type type,
+ u32 kfunc_btf_id)
+{
+ struct btf_id_set *set;
+
+ if (WARN_ON_ONCE(hook >= _BTF_KFUNC_HOOK_MAX || type >= _BTF_KFUNC_TYPE_MAX))
+ return false;
+ if (!btf->kfunc_set_tab)
+ return false;
+ set = btf->kfunc_set_tab->sets[hook][type];
+ if (!set)
+ return false;
+ return btf_id_set_contains(set, kfunc_btf_id);
+}
+
+bool btf_kfunc_id_set_contains(const struct btf *btf,
+ enum bpf_prog_type prog_type,
+ enum btf_kfunc_type type,
+ u32 kfunc_btf_id)
+{
+ enum btf_kfunc_hook hook;
+
+ switch (prog_type) {
+ case BPF_PROG_TYPE_XDP:
+ hook = BTF_KFUNC_HOOK_XDP;
+ break;
+ case BPF_PROG_TYPE_SCHED_CLS:
+ hook = BTF_KFUNC_HOOK_TC;
+ break;
+ case BPF_PROG_TYPE_STRUCT_OPS:
+ hook = BTF_KFUNC_HOOK_STRUCT_OPS;
+ break;
+ default:
+ return false;
+ }
+
+ return __btf_kfunc_id_set_contains(btf, hook, type, kfunc_btf_id);
+}
+
struct btf *btf_parse_vmlinux(void)
{
struct btf_verifier_env *env = NULL;
@@ -4725,6 +4985,10 @@ struct btf *btf_parse_vmlinux(void)
bpf_struct_ops_init(btf, log);
+ err = btf_parse_kfunc_sets(btf, NULL, log);
+ if (err < 0)
+ goto errout;
+
refcount_set(&btf->refcnt, 1);
err = btf_alloc_id(btf);
@@ -4737,6 +5001,7 @@ struct btf *btf_parse_vmlinux(void)
errout:
btf_verifier_env_free(env);
if (btf) {
+ btf_free_kfunc_set_tab(btf->kfunc_set_tab);
kvfree(btf->types);
kfree(btf);
}
@@ -4745,7 +5010,8 @@ struct btf *btf_parse_vmlinux(void)
#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
-static struct btf *btf_parse_module(const char *module_name, const void *data, unsigned int data_size)
+static struct btf *btf_parse_module(struct module *mod, const char *module_name,
+ const void *data, unsigned int data_size)
{
struct btf_verifier_env *env = NULL;
struct bpf_verifier_log *log;
@@ -4800,6 +5066,10 @@ static struct btf *btf_parse_module(const char *module_name, const void *data, u
if (err)
goto errout;
+ err = btf_parse_kfunc_sets(btf, mod, log);
+ if (err)
+ goto errout;
+
btf_verifier_env_free(env);
refcount_set(&btf->refcnt, 1);
return btf;
@@ -4807,6 +5077,7 @@ static struct btf *btf_parse_module(const char *module_name, const void *data, u
errout:
btf_verifier_env_free(env);
if (btf) {
+ btf_free_kfunc_set_tab(btf->kfunc_set_tab);
kvfree(btf->data);
kvfree(btf->types);
kfree(btf);
@@ -6243,7 +6514,7 @@ static int btf_module_notify(struct notifier_block *nb, unsigned long op,
err = -ENOMEM;
goto out;
}
- btf = btf_parse_module(mod->name, mod->btf_data, mod->btf_data_size);
+ btf = btf_parse_module(mod, mod->name, mod->btf_data, mod->btf_data_size);
if (IS_ERR(btf)) {
pr_warn("failed to validate module [%s] BTF: %ld\n",
mod->name, PTR_ERR(btf));
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH bpf-next v5 2/9] bpf: Prepare kfunc BTF ID sets when parsing kernel BTF
2021-12-30 2:36 ` [PATCH bpf-next v5 2/9] bpf: Prepare kfunc BTF ID sets when parsing kernel BTF Kumar Kartikeya Dwivedi
@ 2021-12-31 2:23 ` Alexei Starovoitov
2021-12-31 3:45 ` Kumar Kartikeya Dwivedi
0 siblings, 1 reply; 16+ messages in thread
From: Alexei Starovoitov @ 2021-12-31 2:23 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, netdev, netfilter-devel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, Maxim Mikityanskiy,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen
On Thu, Dec 30, 2021 at 08:06:58AM +0530, Kumar Kartikeya Dwivedi wrote:
>
> The 'hook' is one of the many program types, e.g. XDP and TC/SCHED_CLS,
> STRUCT_OPS, and 'types' are check (allowed or not), acquire, release,
> and ret_null (with PTR_TO_BTF_ID_OR_NULL return type).
>
> A maximum of BTF_KFUNC_SET_MAX_CNT (32) kfunc BTF IDs are permitted in a
> set of certain hook and type. They are also allocated on demand, and
> otherwise set as NULL.
>
> A new btf_kfunc_id_set_contains function is exposed for use in verifier,
> this new method is faster than the existing list searching method, and
> is also automatic. It also lets other code not care whether the set is
> unallocated or not.
>
> Next commit will update the kernel users to make use of this
> infrastructure.
>
> Finally, add __maybe_unused annotation for BTF ID macros for the
> !CONFIG_DEBUG_INFO_BTF case , so that they don't produce warnings during
> build time.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> fixup maybe_unused
> ---
> include/linux/btf.h | 25 ++++
> include/linux/btf_ids.h | 20 ++-
> kernel/bpf/btf.c | 275 +++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 312 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 0c74348cbc9d..48ac2dc437a2 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -300,6 +300,21 @@ static inline const struct btf_var_secinfo *btf_type_var_secinfo(
> return (const struct btf_var_secinfo *)(t + 1);
> }
>
> +enum btf_kfunc_hook {
> + BTF_KFUNC_HOOK_XDP,
> + BTF_KFUNC_HOOK_TC,
> + BTF_KFUNC_HOOK_STRUCT_OPS,
> + _BTF_KFUNC_HOOK_MAX,
> +};
> +
> +enum btf_kfunc_type {
> + BTF_KFUNC_TYPE_CHECK,
> + BTF_KFUNC_TYPE_ACQUIRE,
> + BTF_KFUNC_TYPE_RELEASE,
> + BTF_KFUNC_TYPE_RET_NULL,
> + _BTF_KFUNC_TYPE_MAX,
> +};
> +
> #ifdef CONFIG_BPF_SYSCALL
> struct bpf_prog;
>
> @@ -307,6 +322,9 @@ const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
> const char *btf_name_by_offset(const struct btf *btf, u32 offset);
> struct btf *btf_parse_vmlinux(void);
> struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog);
> +bool btf_kfunc_id_set_contains(const struct btf *btf,
> + enum bpf_prog_type prog_type,
> + enum btf_kfunc_type type, u32 kfunc_btf_id);
> #else
> static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
> u32 type_id)
> @@ -318,6 +336,13 @@ static inline const char *btf_name_by_offset(const struct btf *btf,
> {
> return NULL;
> }
> +static inline bool btf_kfunc_id_set_contains(const struct btf *btf,
> + enum bpf_prog_type prog_type,
> + enum btf_kfunc_type type,
> + u32 kfunc_btf_id)
> +{
> + return false;
> +}
> #endif
>
> struct kfunc_btf_id_set {
> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index 919c0fde1c51..835fbf626ef1 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -11,6 +11,7 @@ struct btf_id_set {
> #ifdef CONFIG_DEBUG_INFO_BTF
>
> #include <linux/compiler.h> /* for __PASTE */
> +#include <linux/compiler_attributes.h> /* for __maybe_unused */
>
> /*
> * Following macros help to define lists of BTF IDs placed
> @@ -144,17 +145,24 @@ asm( \
> ".popsection; \n"); \
> extern struct btf_id_set name;
>
> +#define BTF_KFUNC_SET_START(hook, type, name) \
> + BTF_SET_START(btf_kfunc_set_##hook##_##type##_##name)
> +#define BTF_KFUNC_SET_END(hook, type, name) \
> + BTF_SET_END(btf_kfunc_set_##hook##_##type##_##name)
> +
> #else
>
> -#define BTF_ID_LIST(name) static u32 name[5];
> +#define BTF_ID_LIST(name) static u32 __maybe_unused name[5];
> #define BTF_ID(prefix, name)
> #define BTF_ID_UNUSED
> -#define BTF_ID_LIST_GLOBAL(name, n) u32 name[n];
> -#define BTF_ID_LIST_SINGLE(name, prefix, typename) static u32 name[1];
> -#define BTF_ID_LIST_GLOBAL_SINGLE(name, prefix, typename) u32 name[1];
> -#define BTF_SET_START(name) static struct btf_id_set name = { 0 };
> -#define BTF_SET_START_GLOBAL(name) static struct btf_id_set name = { 0 };
> +#define BTF_ID_LIST_GLOBAL(name, n) u32 __maybe_unused name[n];
> +#define BTF_ID_LIST_SINGLE(name, prefix, typename) static u32 __maybe_unused name[1];
> +#define BTF_ID_LIST_GLOBAL_SINGLE(name, prefix, typename) u32 __maybe_unused name[1];
> +#define BTF_SET_START(name) static struct btf_id_set __maybe_unused name = { 0 };
> +#define BTF_SET_START_GLOBAL(name) static struct btf_id_set __maybe_unused name = { 0 };
> #define BTF_SET_END(name)
> +#define BTF_KFUNC_SET_START(hook, type, name) BTF_SET_START(name)
> +#define BTF_KFUNC_SET_END(hook, type, name) BTF_SET_END(name)
>
> #endif /* CONFIG_DEBUG_INFO_BTF */
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 33bb8ae4a804..c03c7b5a417c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -1,6 +1,8 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /* Copyright (c) 2018 Facebook */
>
> +#include <linux/kallsyms.h>
> +#include <linux/module.h>
> #include <uapi/linux/btf.h>
> #include <uapi/linux/bpf.h>
> #include <uapi/linux/bpf_perf_event.h>
> @@ -198,6 +200,8 @@
> DEFINE_IDR(btf_idr);
> DEFINE_SPINLOCK(btf_idr_lock);
>
> +struct btf_kfunc_set_tab;
> +
> struct btf {
> void *data;
> struct btf_type **types;
> @@ -212,6 +216,7 @@ struct btf {
> refcount_t refcnt;
> u32 id;
> struct rcu_head rcu;
> + struct btf_kfunc_set_tab *kfunc_set_tab;
>
> /* split BTF support */
> struct btf *base_btf;
> @@ -221,6 +226,31 @@ struct btf {
> bool kernel_btf;
> };
>
> +#define BTF_KFUNC_SET_PREFIX "btf_kfunc_set_"
> +
> +BTF_ID_LIST_SINGLE(btf_id_set_id, struct, btf_id_set)
> +
> +static const char *kfunc_hook_str[_BTF_KFUNC_HOOK_MAX] = {
> + [BTF_KFUNC_HOOK_XDP] = "xdp_",
> + [BTF_KFUNC_HOOK_TC] = "tc_",
> + [BTF_KFUNC_HOOK_STRUCT_OPS] = "struct_ops_",
> +};
> +
> +static const char *kfunc_type_str[_BTF_KFUNC_TYPE_MAX] = {
> + [BTF_KFUNC_TYPE_CHECK] = "check_",
> + [BTF_KFUNC_TYPE_ACQUIRE] = "acquire_",
> + [BTF_KFUNC_TYPE_RELEASE] = "release_",
> + [BTF_KFUNC_TYPE_RET_NULL] = "ret_null_",
> +};
> +
> +enum {
> + BTF_KFUNC_SET_MAX_CNT = 32,
> +};
> +
> +struct btf_kfunc_set_tab {
> + struct btf_id_set *sets[_BTF_KFUNC_HOOK_MAX][_BTF_KFUNC_TYPE_MAX];
> +};
> +
> enum verifier_phase {
> CHECK_META,
> CHECK_TYPE,
> @@ -1531,8 +1561,21 @@ static void btf_free_id(struct btf *btf)
> spin_unlock_irqrestore(&btf_idr_lock, flags);
> }
>
> +static void btf_free_kfunc_set_tab(struct btf_kfunc_set_tab *tab)
> +{
> + int hook, type;
> +
> + if (!tab)
> + return;
> + for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++) {
> + for (type = 0; type < ARRAY_SIZE(tab->sets[0]); type++)
> + kfree(tab->sets[hook][type]);
> + }
> +}
> +
> static void btf_free(struct btf *btf)
> {
> + btf_free_kfunc_set_tab(btf->kfunc_set_tab);
> kvfree(btf->types);
> kvfree(btf->resolved_sizes);
> kvfree(btf->resolved_ids);
> @@ -4675,6 +4718,223 @@ static int btf_translate_to_vmlinux(struct bpf_verifier_log *log,
> BTF_ID_LIST(bpf_ctx_convert_btf_id)
> BTF_ID(struct, bpf_ctx_convert)
>
> +struct btf_parse_kfunc_data {
> + struct btf *btf;
> + struct bpf_verifier_log *log;
> +};
> +
> +static int btf_populate_kfunc_sets(struct btf *btf,
> + struct bpf_verifier_log *log,
> + enum btf_kfunc_hook hook,
> + enum btf_kfunc_type type,
> + struct btf_id_set *add_set)
> +{
> + struct btf_id_set *set, *tmp_set;
> + struct btf_kfunc_set_tab *tab;
> + u32 set_cnt;
> + int ret;
> +
> + if (WARN_ON_ONCE(hook >= _BTF_KFUNC_HOOK_MAX || type >= _BTF_KFUNC_TYPE_MAX))
> + return -EINVAL;
> + if (!add_set->cnt)
> + return 0;
> +
> + tab = btf->kfunc_set_tab;
> + if (!tab) {
> + tab = kzalloc(sizeof(*tab), GFP_KERNEL | __GFP_NOWARN);
> + if (!tab)
> + return -ENOMEM;
> + btf->kfunc_set_tab = tab;
> + }
> +
> + set = tab->sets[hook][type];
> + set_cnt = set ? set->cnt : 0;
> +
> + if (set_cnt > U32_MAX - add_set->cnt) {
> + ret = -EOVERFLOW;
> + goto end;
> + }
> +
> + if (set_cnt + add_set->cnt > BTF_KFUNC_SET_MAX_CNT) {
> + bpf_log(log, "max kfunc (%d) for hook '%s' type '%s' exceeded\n",
> + BTF_KFUNC_SET_MAX_CNT, kfunc_hook_str[hook], kfunc_type_str[type]);
> + ret = -E2BIG;
> + goto end;
> + }
> +
> + /* Grow set */
> + tmp_set = krealloc(set, offsetof(struct btf_id_set, ids[set_cnt + add_set->cnt]),
> + GFP_KERNEL | __GFP_NOWARN);
> + if (!tmp_set) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + /* For newly allocated set, initialize set->cnt to 0 */
> + if (!set)
> + tmp_set->cnt = 0;
> +
> + tab->sets[hook][type] = tmp_set;
> + set = tmp_set;
> +
> + /* Concatenate the two sets */
> + memcpy(set->ids + set->cnt, add_set->ids, add_set->cnt * sizeof(set->ids[0]));
> + set->cnt += add_set->cnt;
> +
> + return 0;
> +end:
> + btf_free_kfunc_set_tab(tab);
> + btf->kfunc_set_tab = NULL;
> + return ret;
> +}
> +
> +static int btf_kfunc_ids_cmp(const void *a, const void *b)
> +{
> + const u32 *id1 = a;
> + const u32 *id2 = b;
> +
> + if (*id1 < *id2)
> + return -1;
> + if (*id1 > *id2)
> + return 1;
> + return 0;
> +}
> +
> +static int btf_parse_kfunc_sets_cb(void *data, const char *symbol_name,
> + struct module *mod,
> + unsigned long symbol_value)
> +{
> + int pfx_size = sizeof(BTF_KFUNC_SET_PREFIX) - 1;
> + struct btf_id_set *set = (void *)symbol_value;
> + struct btf_parse_kfunc_data *bdata = data;
> + const char *orig_name = symbol_name;
> + int i, hook, type;
> +
> + BUILD_BUG_ON(ARRAY_SIZE(kfunc_hook_str) != _BTF_KFUNC_HOOK_MAX);
> + BUILD_BUG_ON(ARRAY_SIZE(kfunc_type_str) != _BTF_KFUNC_TYPE_MAX);
> +
> + if (strncmp(symbol_name, BTF_KFUNC_SET_PREFIX, pfx_size))
> + return 0;
> +
> + /* Identify hook */
> + symbol_name += pfx_size;
> + if (!*symbol_name) {
> + bpf_log(bdata->log, "incomplete kfunc btf_id_set specification: %s\n", orig_name);
> + return -EINVAL;
> + }
> + for (i = 0; i < ARRAY_SIZE(kfunc_hook_str); i++) {
> + pfx_size = strlen(kfunc_hook_str[i]);
> + if (strncmp(symbol_name, kfunc_hook_str[i], pfx_size))
> + continue;
> + break;
> + }
> + if (i == ARRAY_SIZE(kfunc_hook_str)) {
> + bpf_log(bdata->log, "invalid hook '%s' for kfunc_btf_id_set %s\n", symbol_name,
> + orig_name);
> + return -EINVAL;
> + }
> + hook = i;
> +
> + /* Identify type */
> + symbol_name += pfx_size;
> + if (!*symbol_name) {
> + bpf_log(bdata->log, "incomplete kfunc btf_id_set specification: %s\n", orig_name);
> + return -EINVAL;
> + }
> + for (i = 0; i < ARRAY_SIZE(kfunc_type_str); i++) {
> + pfx_size = strlen(kfunc_type_str[i]);
> + if (strncmp(symbol_name, kfunc_type_str[i], pfx_size))
> + continue;
> + break;
> + }
> + if (i == ARRAY_SIZE(kfunc_type_str)) {
> + bpf_log(bdata->log, "invalid type '%s' for kfunc_btf_id_set %s\n", symbol_name,
> + orig_name);
> + return -EINVAL;
> + }
> + type = i;
> +
> + return btf_populate_kfunc_sets(bdata->btf, bdata->log, hook, type, set);
I really like the direction taken by patches 2 and 3.
I think we can save the module_kallsyms_on_each_symbol loop though.
The registration mechanism, like:
register_kfunc_btf_id_set(&prog_test_kfunc_list, &bpf_testmod_kfunc_btf_set);
doesn't have to be complete removed.
It can replaced with a sequence of calls:
btf_populate_kfunc_sets(btf, hook, type, set);
from __init of the module.
The module knows its 'hook' and 'type' and set==&bpf_testmod_kfunc_btf_set.
The bpf_testmod_init() can call btf_populate_kfunc_sets() multiple
times to popualte sets into different hooks and types.
There is no need to 'unregister' any more.
And the patch 1 will no longer be necessary, since we don't need to iterate
every symbol of the module with module_kallsyms_on_each_symbol().
No need to standardize on the prefix and kfunc_[hook|type]_str,
though it's probably good idea anyway across module BTF sets.
The main disadvantage is that we lose 'log' in btf_populate_kfunc_sets(),
since __init of the module cannot have verifier log at that point.
But it can stay as 'ret = -E2BIG;' without bpf_log() and module registration
will fail in such case or we just warn inside __init if btf_populate_kfunc_sets
fails in the rare case.
wdyt?
> +}
> +
> +static int btf_parse_kfunc_sets(struct btf *btf, struct module *mod,
> + struct bpf_verifier_log *log)
> +{
> + struct btf_parse_kfunc_data data = { .btf = btf, .log = log, };
> + struct btf_kfunc_set_tab *tab;
> + int hook, type, ret;
> +
> + if (!btf_is_kernel(btf))
> + return -EINVAL;
> + if (WARN_ON_ONCE(btf_is_module(btf) && !mod)) {
> + bpf_log(log, "btf internal error: no module for module BTF %s\n", btf->name);
> + return -EFAULT;
> + }
> + if (mod)
> + ret = module_kallsyms_on_each_symbol(mod, btf_parse_kfunc_sets_cb, &data);
> + else
> + ret = kallsyms_on_each_symbol(btf_parse_kfunc_sets_cb, &data);
> +
> + tab = btf->kfunc_set_tab;
> + if (!ret && tab) {
> + /* Sort all populated sets */
> + for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++) {
> + for (type = 0; type < ARRAY_SIZE(tab->sets[0]); type++) {
> + struct btf_id_set *set = tab->sets[hook][type];
> +
> + /* Not all sets may be populated */
> + if (!set)
> + continue;
> + sort(set->ids, set->cnt, sizeof(set->ids[0]), btf_kfunc_ids_cmp,
> + NULL);
Didn't resolve_btfid store ids already sorted?
Why another sort is needed?
Because btf_populate_kfunc_sets() can concatenate the sets?
But if we let __init call it directly the module shouldn't use
the same hook/type combination multiple times with different sets.
So no secondary sorting will be necessary?
> This commit prepares the BTF parsing functions for vmlinux and module
> BTFs to find all kfunc BTF ID sets from the vmlinux and module symbols
> and concatentate all sets into single unified set which is sorted and
> keyed by the 'hook' it is meant for, and 'type' of set.
'sorted by hook' ?
The btf_id_set_contains() need to search it by 'id', so it's sorted by 'id'.
Is it because you're adding mod's IDs to vmlinux IDs and re-sorting them?
I think that's not worth optimizing. The patch 5 is doing:
btf_kfunc_id_set_contains(btf, prog_type, BTF_KFUNC_TYPE_RELEASE|ACQUIRE|RET_NULL, id)
but btf_kfunc_id_set_contains doesn't have to do it in a single bsearch.
The struct btf of the module has base_btf.
So btf_kfunc_id_set_contains() can do bsearch twice. Once in mod's btf sets[type][hook]
and once in vmlinux btf sets[type][hook]
and no secondary sorting will be necessary.
wdyt?
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH bpf-next v5 2/9] bpf: Prepare kfunc BTF ID sets when parsing kernel BTF
2021-12-31 2:23 ` Alexei Starovoitov
@ 2021-12-31 3:45 ` Kumar Kartikeya Dwivedi
2021-12-31 16:56 ` Alexei Starovoitov
0 siblings, 1 reply; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-31 3:45 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, netdev, netfilter-devel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, Maxim Mikityanskiy,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen
On Fri, Dec 31, 2021 at 07:53:07AM IST, Alexei Starovoitov wrote:
> On Thu, Dec 30, 2021 at 08:06:58AM +0530, Kumar Kartikeya Dwivedi wrote:
> > [...]
> > +
> > + /* Identify type */
> > + symbol_name += pfx_size;
> > + if (!*symbol_name) {
> > + bpf_log(bdata->log, "incomplete kfunc btf_id_set specification: %s\n", orig_name);
> > + return -EINVAL;
> > + }
> > + for (i = 0; i < ARRAY_SIZE(kfunc_type_str); i++) {
> > + pfx_size = strlen(kfunc_type_str[i]);
> > + if (strncmp(symbol_name, kfunc_type_str[i], pfx_size))
> > + continue;
> > + break;
> > + }
> > + if (i == ARRAY_SIZE(kfunc_type_str)) {
> > + bpf_log(bdata->log, "invalid type '%s' for kfunc_btf_id_set %s\n", symbol_name,
> > + orig_name);
> > + return -EINVAL;
> > + }
> > + type = i;
> > +
> > + return btf_populate_kfunc_sets(bdata->btf, bdata->log, hook, type, set);
>
> I really like the direction taken by patches 2 and 3.
> I think we can save the module_kallsyms_on_each_symbol loop though.
> The registration mechanism, like:
> register_kfunc_btf_id_set(&prog_test_kfunc_list, &bpf_testmod_kfunc_btf_set);
> doesn't have to be complete removed.
> It can replaced with a sequence of calls:
> btf_populate_kfunc_sets(btf, hook, type, set);
> from __init of the module.
> The module knows its 'hook' and 'type' and set==&bpf_testmod_kfunc_btf_set.
> The bpf_testmod_init() can call btf_populate_kfunc_sets() multiple
> times to popualte sets into different hooks and types.
> There is no need to 'unregister' any more.
> And the patch 1 will no longer be necessary, since we don't need to iterate
> every symbol of the module with module_kallsyms_on_each_symbol().
> No need to standardize on the prefix and kfunc_[hook|type]_str,
> though it's probably good idea anyway across module BTF sets.
> The main disadvantage is that we lose 'log' in btf_populate_kfunc_sets(),
> since __init of the module cannot have verifier log at that point.
> But it can stay as 'ret = -E2BIG;' without bpf_log() and module registration
> will fail in such case or we just warn inside __init if btf_populate_kfunc_sets
> fails in the rare case.
> wdyt?
>
Sounds good, I'll make this change in the next version. Should I also drop
kallsyms_on_each_symbol for vmlinux BTF? I think we can use initcall for it too,
right?
> > +}
> > +
> > +static int btf_parse_kfunc_sets(struct btf *btf, struct module *mod,
> > + struct bpf_verifier_log *log)
> > +{
> > + struct btf_parse_kfunc_data data = { .btf = btf, .log = log, };
> > + struct btf_kfunc_set_tab *tab;
> > + int hook, type, ret;
> > +
> > + if (!btf_is_kernel(btf))
> > + return -EINVAL;
> > + if (WARN_ON_ONCE(btf_is_module(btf) && !mod)) {
> > + bpf_log(log, "btf internal error: no module for module BTF %s\n", btf->name);
> > + return -EFAULT;
> > + }
> > + if (mod)
> > + ret = module_kallsyms_on_each_symbol(mod, btf_parse_kfunc_sets_cb, &data);
> > + else
> > + ret = kallsyms_on_each_symbol(btf_parse_kfunc_sets_cb, &data);
> > +
> > + tab = btf->kfunc_set_tab;
> > + if (!ret && tab) {
> > + /* Sort all populated sets */
> > + for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++) {
> > + for (type = 0; type < ARRAY_SIZE(tab->sets[0]); type++) {
> > + struct btf_id_set *set = tab->sets[hook][type];
> > +
> > + /* Not all sets may be populated */
> > + if (!set)
> > + continue;
> > + sort(set->ids, set->cnt, sizeof(set->ids[0]), btf_kfunc_ids_cmp,
> > + NULL);
>
> Didn't resolve_btfid store ids already sorted?
> Why another sort is needed?
Because it might be possible while iterating over symbols (be it vmlinux or
module), we combine sets like [1, 4, 6] and [2, 3, 5] into [1, 4, 6, 2, 3, 5],
into the set for a certain hook, type, so to enable bsearch we do one final sort
after possible sets have been populated.
> Because btf_populate_kfunc_sets() can concatenate the sets?
> But if we let __init call it directly the module shouldn't use
> the same hook/type combination multiple times with different sets.
> So no secondary sorting will be necessary?
>
Yes, if we make it that only one call per hook/type can be done, then this
shouldn't be needed, but e.g. if each file has a set for some hook and uses late
initcall to do registration, then it will be needed again for the same reason.
We can surely catch the second call (see if tab->[hook][type] != NULL).
> > This commit prepares the BTF parsing functions for vmlinux and module
> > BTFs to find all kfunc BTF ID sets from the vmlinux and module symbols
> > and concatentate all sets into single unified set which is sorted and
> > keyed by the 'hook' it is meant for, and 'type' of set.
>
> 'sorted by hook' ?
> The btf_id_set_contains() need to search it by 'id', so it's sorted by 'id'.
Yeah, it needs a comma after 'sorted' :).
> Is it because you're adding mod's IDs to vmlinux IDs and re-sorting them?
No, I'm not mixing them. We only add same module's/vmlinux's IDs to its struct
btf, then sort each set (for hook,type pair). find_kfunc_desc_btf gives us the
BTF, then we can directly do what is essentially a single btf_id_set_contains
call, so it is not required to research in vmlinux BTF. The BTF associated with
the kfunc call is known.
> I think that's not worth optimizing. The patch 5 is doing:
> btf_kfunc_id_set_contains(btf, prog_type, BTF_KFUNC_TYPE_RELEASE|ACQUIRE|RET_NULL, id)
> but btf_kfunc_id_set_contains doesn't have to do it in a single bsearch.
> The struct btf of the module has base_btf.
> So btf_kfunc_id_set_contains() can do bsearch twice. Once in mod's btf sets[type][hook]
> and once in vmlinux btf sets[type][hook]
> and no secondary sorting will be necessary.
> wdyt?
--
Kartikeya
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH bpf-next v5 2/9] bpf: Prepare kfunc BTF ID sets when parsing kernel BTF
2021-12-31 3:45 ` Kumar Kartikeya Dwivedi
@ 2021-12-31 16:56 ` Alexei Starovoitov
0 siblings, 0 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2021-12-31 16:56 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, netdev, netfilter-devel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, Maxim Mikityanskiy,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen
On Fri, Dec 31, 2021 at 09:15:07AM +0530, Kumar Kartikeya Dwivedi wrote:
> On Fri, Dec 31, 2021 at 07:53:07AM IST, Alexei Starovoitov wrote:
> > On Thu, Dec 30, 2021 at 08:06:58AM +0530, Kumar Kartikeya Dwivedi wrote:
> > > [...]
> > > +
> > > + /* Identify type */
> > > + symbol_name += pfx_size;
> > > + if (!*symbol_name) {
> > > + bpf_log(bdata->log, "incomplete kfunc btf_id_set specification: %s\n", orig_name);
> > > + return -EINVAL;
> > > + }
> > > + for (i = 0; i < ARRAY_SIZE(kfunc_type_str); i++) {
> > > + pfx_size = strlen(kfunc_type_str[i]);
> > > + if (strncmp(symbol_name, kfunc_type_str[i], pfx_size))
> > > + continue;
> > > + break;
> > > + }
> > > + if (i == ARRAY_SIZE(kfunc_type_str)) {
> > > + bpf_log(bdata->log, "invalid type '%s' for kfunc_btf_id_set %s\n", symbol_name,
> > > + orig_name);
> > > + return -EINVAL;
> > > + }
> > > + type = i;
> > > +
> > > + return btf_populate_kfunc_sets(bdata->btf, bdata->log, hook, type, set);
> >
> > I really like the direction taken by patches 2 and 3.
> > I think we can save the module_kallsyms_on_each_symbol loop though.
> > The registration mechanism, like:
> > register_kfunc_btf_id_set(&prog_test_kfunc_list, &bpf_testmod_kfunc_btf_set);
> > doesn't have to be complete removed.
> > It can replaced with a sequence of calls:
> > btf_populate_kfunc_sets(btf, hook, type, set);
> > from __init of the module.
> > The module knows its 'hook' and 'type' and set==&bpf_testmod_kfunc_btf_set.
> > The bpf_testmod_init() can call btf_populate_kfunc_sets() multiple
> > times to popualte sets into different hooks and types.
> > There is no need to 'unregister' any more.
> > And the patch 1 will no longer be necessary, since we don't need to iterate
> > every symbol of the module with module_kallsyms_on_each_symbol().
> > No need to standardize on the prefix and kfunc_[hook|type]_str,
> > though it's probably good idea anyway across module BTF sets.
> > The main disadvantage is that we lose 'log' in btf_populate_kfunc_sets(),
> > since __init of the module cannot have verifier log at that point.
> > But it can stay as 'ret = -E2BIG;' without bpf_log() and module registration
> > will fail in such case or we just warn inside __init if btf_populate_kfunc_sets
> > fails in the rare case.
> > wdyt?
> >
>
> Sounds good, I'll make this change in the next version. Should I also drop
> kallsyms_on_each_symbol for vmlinux BTF? I think we can use initcall for it too,
> right?
Yep. Of course. For consistency.
> > > +}
> > > +
> > > +static int btf_parse_kfunc_sets(struct btf *btf, struct module *mod,
> > > + struct bpf_verifier_log *log)
> > > +{
> > > + struct btf_parse_kfunc_data data = { .btf = btf, .log = log, };
> > > + struct btf_kfunc_set_tab *tab;
> > > + int hook, type, ret;
> > > +
> > > + if (!btf_is_kernel(btf))
> > > + return -EINVAL;
> > > + if (WARN_ON_ONCE(btf_is_module(btf) && !mod)) {
> > > + bpf_log(log, "btf internal error: no module for module BTF %s\n", btf->name);
> > > + return -EFAULT;
> > > + }
> > > + if (mod)
> > > + ret = module_kallsyms_on_each_symbol(mod, btf_parse_kfunc_sets_cb, &data);
> > > + else
> > > + ret = kallsyms_on_each_symbol(btf_parse_kfunc_sets_cb, &data);
> > > +
> > > + tab = btf->kfunc_set_tab;
> > > + if (!ret && tab) {
> > > + /* Sort all populated sets */
> > > + for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++) {
> > > + for (type = 0; type < ARRAY_SIZE(tab->sets[0]); type++) {
> > > + struct btf_id_set *set = tab->sets[hook][type];
> > > +
> > > + /* Not all sets may be populated */
> > > + if (!set)
> > > + continue;
> > > + sort(set->ids, set->cnt, sizeof(set->ids[0]), btf_kfunc_ids_cmp,
> > > + NULL);
> >
> > Didn't resolve_btfid store ids already sorted?
> > Why another sort is needed?
>
> Because it might be possible while iterating over symbols (be it vmlinux or
> module), we combine sets like [1, 4, 6] and [2, 3, 5] into [1, 4, 6, 2, 3, 5],
> into the set for a certain hook, type, so to enable bsearch we do one final sort
> after possible sets have been populated.
>
> > Because btf_populate_kfunc_sets() can concatenate the sets?
> > But if we let __init call it directly the module shouldn't use
> > the same hook/type combination multiple times with different sets.
> > So no secondary sorting will be necessary?
> >
>
> Yes, if we make it that only one call per hook/type can be done, then this
> shouldn't be needed, but e.g. if each file has a set for some hook and uses late
> initcall to do registration, then it will be needed again for the same reason.
You mean when module has multiple files and these sets have to be in different files?
> We can surely catch the second call (see if tab->[hook][type] != NULL).
Good idea. Let's do this for now.
One .c per module with such sets is fine, imo.
> > > This commit prepares the BTF parsing functions for vmlinux and module
> > > BTFs to find all kfunc BTF ID sets from the vmlinux and module symbols
> > > and concatentate all sets into single unified set which is sorted and
> > > keyed by the 'hook' it is meant for, and 'type' of set.
> >
> > 'sorted by hook' ?
> > The btf_id_set_contains() need to search it by 'id', so it's sorted by 'id'.
>
> Yeah, it needs a comma after 'sorted' :).
>
> > Is it because you're adding mod's IDs to vmlinux IDs and re-sorting them?
>
> No, I'm not mixing them. We only add same module's/vmlinux's IDs to its struct
> btf, then sort each set (for hook,type pair). find_kfunc_desc_btf gives us the
> BTF, then we can directly do what is essentially a single btf_id_set_contains
> call, so it is not required to research in vmlinux BTF. The BTF associated with
> the kfunc call is known.
Got it. Ok.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH bpf-next v5 3/9] bpf: Remove check_kfunc_call callback and old kfunc BTF ID API
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
2021-12-30 2:36 ` [PATCH bpf-next v5 1/9] kernel: Add kallsyms_on_each_symbol variant for single module Kumar Kartikeya Dwivedi
2021-12-30 2:36 ` [PATCH bpf-next v5 2/9] bpf: Prepare kfunc BTF ID sets when parsing kernel BTF Kumar Kartikeya Dwivedi
@ 2021-12-30 2:36 ` Kumar Kartikeya Dwivedi
2021-12-30 2:37 ` [PATCH bpf-next v5 4/9] bpf: Introduce mem, size argument pair support for kfunc Kumar Kartikeya Dwivedi
` (6 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:36 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
Completely remove the old code for check_kfunc_call to help it work
with modules.
The previous commit finds all symbols in vmlinux or modules while
parsing and preparing their BTF, and concatenates all related sets
organized by the hook and the type. Then, they are sorted to enable
bsearch using btf_id_set_contains.
Also, since we don't need the 'owner' module anywhere when doing
check_kfunc_call, drop the 'btf_modp' module parameter from
find_kfunc_desc_btf.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/bpf.h | 8 ----
include/linux/btf.h | 44 -----------------
kernel/bpf/btf.c | 48 -------------------
kernel/bpf/verifier.c | 20 ++++----
net/bpf/test_run.c | 11 +----
net/core/filter.c | 1 -
net/ipv4/bpf_tcp_ca.c | 12 +----
net/ipv4/tcp_bbr.c | 15 ++----
net/ipv4/tcp_cubic.c | 15 ++----
net/ipv4/tcp_dctcp.c | 15 ++----
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 15 ++----
11 files changed, 24 insertions(+), 180 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 26753139d5b4..e381aeeffdd2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -573,7 +573,6 @@ struct bpf_verifier_ops {
const struct btf_type *t, int off, int size,
enum bpf_access_type atype,
u32 *next_btf_id);
- bool (*check_kfunc_call)(u32 kfunc_btf_id, struct module *owner);
};
struct bpf_prog_offload_ops {
@@ -1719,7 +1718,6 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
const union bpf_attr *kattr,
union bpf_attr __user *uattr);
-bool bpf_prog_test_check_kfunc_call(u32 kfunc_id, struct module *owner);
bool btf_ctx_access(int off, int size, enum bpf_access_type type,
const struct bpf_prog *prog,
struct bpf_insn_access_aux *info);
@@ -1971,12 +1969,6 @@ static inline int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
return -ENOTSUPP;
}
-static inline bool bpf_prog_test_check_kfunc_call(u32 kfunc_id,
- struct module *owner)
-{
- return false;
-}
-
static inline void bpf_map_put(struct bpf_map *map)
{
}
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 48ac2dc437a2..b0283a1778c3 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -345,48 +345,4 @@ static inline bool btf_kfunc_id_set_contains(const struct btf *btf,
}
#endif
-struct kfunc_btf_id_set {
- struct list_head list;
- struct btf_id_set *set;
- struct module *owner;
-};
-
-struct kfunc_btf_id_list {
- struct list_head list;
- struct mutex mutex;
-};
-
-#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
-void register_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
- struct kfunc_btf_id_set *s);
-void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
- struct kfunc_btf_id_set *s);
-bool bpf_check_mod_kfunc_call(struct kfunc_btf_id_list *klist, u32 kfunc_id,
- struct module *owner);
-
-extern struct kfunc_btf_id_list bpf_tcp_ca_kfunc_list;
-extern struct kfunc_btf_id_list prog_test_kfunc_list;
-#else
-static inline void register_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
- struct kfunc_btf_id_set *s)
-{
-}
-static inline void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
- struct kfunc_btf_id_set *s)
-{
-}
-static inline bool bpf_check_mod_kfunc_call(struct kfunc_btf_id_list *klist,
- u32 kfunc_id, struct module *owner)
-{
- return false;
-}
-
-static struct kfunc_btf_id_list bpf_tcp_ca_kfunc_list __maybe_unused;
-static struct kfunc_btf_id_list prog_test_kfunc_list __maybe_unused;
-#endif
-
-#define DEFINE_KFUNC_BTF_ID_SET(set, name) \
- struct kfunc_btf_id_set name = { LIST_HEAD_INIT(name.list), (set), \
- THIS_MODULE }
-
#endif
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index c03c7b5a417c..f5b3049a56e0 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6687,54 +6687,6 @@ BTF_ID_LIST_GLOBAL(btf_tracing_ids, MAX_BTF_TRACING_TYPE)
BTF_TRACING_TYPE_xxx
#undef BTF_TRACING_TYPE
-/* BTF ID set registration API for modules */
-
-#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
-
-void register_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
- struct kfunc_btf_id_set *s)
-{
- mutex_lock(&l->mutex);
- list_add(&s->list, &l->list);
- mutex_unlock(&l->mutex);
-}
-EXPORT_SYMBOL_GPL(register_kfunc_btf_id_set);
-
-void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
- struct kfunc_btf_id_set *s)
-{
- mutex_lock(&l->mutex);
- list_del_init(&s->list);
- mutex_unlock(&l->mutex);
-}
-EXPORT_SYMBOL_GPL(unregister_kfunc_btf_id_set);
-
-bool bpf_check_mod_kfunc_call(struct kfunc_btf_id_list *klist, u32 kfunc_id,
- struct module *owner)
-{
- struct kfunc_btf_id_set *s;
-
- mutex_lock(&klist->mutex);
- list_for_each_entry(s, &klist->list, list) {
- if (s->owner == owner && btf_id_set_contains(s->set, kfunc_id)) {
- mutex_unlock(&klist->mutex);
- return true;
- }
- }
- mutex_unlock(&klist->mutex);
- return false;
-}
-
-#define DEFINE_KFUNC_BTF_ID_LIST(name) \
- struct kfunc_btf_id_list name = { LIST_HEAD_INIT(name.list), \
- __MUTEX_INITIALIZER(name.mutex) }; \
- EXPORT_SYMBOL_GPL(name)
-
-DEFINE_KFUNC_BTF_ID_LIST(bpf_tcp_ca_kfunc_list);
-DEFINE_KFUNC_BTF_ID_LIST(prog_test_kfunc_list);
-
-#endif
-
int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
const struct btf *targ_btf, __u32 targ_id)
{
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ca5cd0de804c..e9503192101f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1735,7 +1735,7 @@ find_kfunc_desc(const struct bpf_prog *prog, u32 func_id, u16 offset)
}
static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
- s16 offset, struct module **btf_modp)
+ s16 offset)
{
struct bpf_kfunc_btf kf_btf = { .offset = offset };
struct bpf_kfunc_btf_tab *tab;
@@ -1789,8 +1789,6 @@ static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
kfunc_btf_cmp_by_off, NULL);
}
- if (btf_modp)
- *btf_modp = b->module;
return b->btf;
}
@@ -1807,8 +1805,7 @@ void bpf_free_kfunc_btf_tab(struct bpf_kfunc_btf_tab *tab)
}
static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env,
- u32 func_id, s16 offset,
- struct module **btf_modp)
+ u32 func_id, s16 offset)
{
if (offset) {
if (offset < 0) {
@@ -1819,7 +1816,7 @@ static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env,
return ERR_PTR(-EINVAL);
}
- return __find_kfunc_desc_btf(env, offset, btf_modp);
+ return __find_kfunc_desc_btf(env, offset);
}
return btf_vmlinux ?: ERR_PTR(-ENOENT);
}
@@ -1882,7 +1879,7 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
prog_aux->kfunc_btf_tab = btf_tab;
}
- desc_btf = find_kfunc_desc_btf(env, func_id, offset, NULL);
+ desc_btf = find_kfunc_desc_btf(env, func_id, offset);
if (IS_ERR(desc_btf)) {
verbose(env, "failed to find BTF for kernel function\n");
return PTR_ERR(desc_btf);
@@ -2343,7 +2340,7 @@ static const char *disasm_kfunc_name(void *data, const struct bpf_insn *insn)
if (insn->src_reg != BPF_PSEUDO_KFUNC_CALL)
return NULL;
- desc_btf = find_kfunc_desc_btf(data, insn->imm, insn->off, NULL);
+ desc_btf = find_kfunc_desc_btf(data, insn->imm, insn->off);
if (IS_ERR(desc_btf))
return "<error>";
@@ -6804,7 +6801,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
struct bpf_reg_state *regs = cur_regs(env);
const char *func_name, *ptr_type_name;
u32 i, nargs, func_id, ptr_type_id;
- struct module *btf_mod = NULL;
const struct btf_param *args;
struct btf *desc_btf;
int err;
@@ -6813,7 +6809,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
if (!insn->imm)
return 0;
- desc_btf = find_kfunc_desc_btf(env, insn->imm, insn->off, &btf_mod);
+ desc_btf = find_kfunc_desc_btf(env, insn->imm, insn->off);
if (IS_ERR(desc_btf))
return PTR_ERR(desc_btf);
@@ -6822,8 +6818,8 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
func_name = btf_name_by_offset(desc_btf, func->name_off);
func_proto = btf_type_by_id(desc_btf, func->type);
- if (!env->ops->check_kfunc_call ||
- !env->ops->check_kfunc_call(func_id, btf_mod)) {
+ if (!btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog),
+ BTF_KFUNC_TYPE_CHECK, func_id)) {
verbose(env, "calling kernel function %s is not allowed\n",
func_name);
return -EACCES;
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 46dd95755967..2ccd567aa514 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -236,18 +236,11 @@ __diag_pop();
ALLOW_ERROR_INJECTION(bpf_modify_return_test, ERRNO);
-BTF_SET_START(test_sk_kfunc_ids)
+BTF_KFUNC_SET_START(tc, check, test_sk_kfunc_ids)
BTF_ID(func, bpf_kfunc_call_test1)
BTF_ID(func, bpf_kfunc_call_test2)
BTF_ID(func, bpf_kfunc_call_test3)
-BTF_SET_END(test_sk_kfunc_ids)
-
-bool bpf_prog_test_check_kfunc_call(u32 kfunc_id, struct module *owner)
-{
- if (btf_id_set_contains(&test_sk_kfunc_ids, kfunc_id))
- return true;
- return bpf_check_mod_kfunc_call(&prog_test_kfunc_list, kfunc_id, owner);
-}
+BTF_KFUNC_SET_END(tc, check, test_sk_kfunc_ids)
static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
u32 headroom, u32 tailroom)
diff --git a/net/core/filter.c b/net/core/filter.c
index 606ab5a98a1a..404a9530ed25 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10001,7 +10001,6 @@ const struct bpf_verifier_ops tc_cls_act_verifier_ops = {
.convert_ctx_access = tc_cls_act_convert_ctx_access,
.gen_prologue = tc_cls_act_prologue,
.gen_ld_abs = bpf_gen_ld_abs,
- .check_kfunc_call = bpf_prog_test_check_kfunc_call,
};
const struct bpf_prog_ops tc_cls_act_prog_ops = {
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
index de610cb83694..468fe66220f3 100644
--- a/net/ipv4/bpf_tcp_ca.c
+++ b/net/ipv4/bpf_tcp_ca.c
@@ -212,26 +212,18 @@ bpf_tcp_ca_get_func_proto(enum bpf_func_id func_id,
}
}
-BTF_SET_START(bpf_tcp_ca_kfunc_ids)
+BTF_KFUNC_SET_START(struct_ops, check, bpf_tcp_ca)
BTF_ID(func, tcp_reno_ssthresh)
BTF_ID(func, tcp_reno_cong_avoid)
BTF_ID(func, tcp_reno_undo_cwnd)
BTF_ID(func, tcp_slow_start)
BTF_ID(func, tcp_cong_avoid_ai)
-BTF_SET_END(bpf_tcp_ca_kfunc_ids)
-
-static bool bpf_tcp_ca_check_kfunc_call(u32 kfunc_btf_id, struct module *owner)
-{
- if (btf_id_set_contains(&bpf_tcp_ca_kfunc_ids, kfunc_btf_id))
- return true;
- return bpf_check_mod_kfunc_call(&bpf_tcp_ca_kfunc_list, kfunc_btf_id, owner);
-}
+BTF_KFUNC_SET_END(struct_ops, check, bpf_tcp_ca)
static const struct bpf_verifier_ops bpf_tcp_ca_verifier_ops = {
.get_func_proto = bpf_tcp_ca_get_func_proto,
.is_valid_access = bpf_tcp_ca_is_valid_access,
.btf_struct_access = bpf_tcp_ca_btf_struct_access,
- .check_kfunc_call = bpf_tcp_ca_check_kfunc_call,
};
static int bpf_tcp_ca_init_member(const struct btf_type *t,
diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
index ec5550089b4d..f65582f3795c 100644
--- a/net/ipv4/tcp_bbr.c
+++ b/net/ipv4/tcp_bbr.c
@@ -1154,7 +1154,7 @@ static struct tcp_congestion_ops tcp_bbr_cong_ops __read_mostly = {
.set_state = bbr_set_state,
};
-BTF_SET_START(tcp_bbr_kfunc_ids)
+BTF_KFUNC_SET_START(struct_ops, check, tcp_bbr)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
BTF_ID(func, bbr_init)
@@ -1167,25 +1167,16 @@ BTF_ID(func, bbr_min_tso_segs)
BTF_ID(func, bbr_set_state)
#endif
#endif
-BTF_SET_END(tcp_bbr_kfunc_ids)
-
-static DEFINE_KFUNC_BTF_ID_SET(&tcp_bbr_kfunc_ids, tcp_bbr_kfunc_btf_set);
+BTF_KFUNC_SET_END(struct_ops, check, tcp_bbr)
static int __init bbr_register(void)
{
- int ret;
-
BUILD_BUG_ON(sizeof(struct bbr) > ICSK_CA_PRIV_SIZE);
- ret = tcp_register_congestion_control(&tcp_bbr_cong_ops);
- if (ret)
- return ret;
- register_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_bbr_kfunc_btf_set);
- return 0;
+ return tcp_register_congestion_control(&tcp_bbr_cong_ops);
}
static void __exit bbr_unregister(void)
{
- unregister_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_bbr_kfunc_btf_set);
tcp_unregister_congestion_control(&tcp_bbr_cong_ops);
}
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index e07837e23b3f..a67bad26b69e 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -485,7 +485,7 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
.name = "cubic",
};
-BTF_SET_START(tcp_cubic_kfunc_ids)
+BTF_KFUNC_SET_START(struct_ops, check, tcp_cubic)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
BTF_ID(func, cubictcp_init)
@@ -496,14 +496,10 @@ BTF_ID(func, cubictcp_cwnd_event)
BTF_ID(func, cubictcp_acked)
#endif
#endif
-BTF_SET_END(tcp_cubic_kfunc_ids)
-
-static DEFINE_KFUNC_BTF_ID_SET(&tcp_cubic_kfunc_ids, tcp_cubic_kfunc_btf_set);
+BTF_KFUNC_SET_END(struct_ops, check, tcp_cubic)
static int __init cubictcp_register(void)
{
- int ret;
-
BUILD_BUG_ON(sizeof(struct bictcp) > ICSK_CA_PRIV_SIZE);
/* Precompute a bunch of the scaling factors that are used per-packet
@@ -534,16 +530,11 @@ static int __init cubictcp_register(void)
/* divide by bic_scale and by constant Srtt (100ms) */
do_div(cube_factor, bic_scale * 10);
- ret = tcp_register_congestion_control(&cubictcp);
- if (ret)
- return ret;
- register_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_cubic_kfunc_btf_set);
- return 0;
+ return tcp_register_congestion_control(&cubictcp);
}
static void __exit cubictcp_unregister(void)
{
- unregister_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_cubic_kfunc_btf_set);
tcp_unregister_congestion_control(&cubictcp);
}
diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
index 0d7ab3cc7b61..b5d7b1b914b3 100644
--- a/net/ipv4/tcp_dctcp.c
+++ b/net/ipv4/tcp_dctcp.c
@@ -238,7 +238,7 @@ static struct tcp_congestion_ops dctcp_reno __read_mostly = {
.name = "dctcp-reno",
};
-BTF_SET_START(tcp_dctcp_kfunc_ids)
+BTF_KFUNC_SET_START(struct_ops, check, tcp_dctcp)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
BTF_ID(func, dctcp_init)
@@ -249,25 +249,16 @@ BTF_ID(func, dctcp_cwnd_undo)
BTF_ID(func, dctcp_state)
#endif
#endif
-BTF_SET_END(tcp_dctcp_kfunc_ids)
-
-static DEFINE_KFUNC_BTF_ID_SET(&tcp_dctcp_kfunc_ids, tcp_dctcp_kfunc_btf_set);
+BTF_KFUNC_SET_END(struct_ops, check, tcp_dctcp)
static int __init dctcp_register(void)
{
- int ret;
-
BUILD_BUG_ON(sizeof(struct dctcp) > ICSK_CA_PRIV_SIZE);
- ret = tcp_register_congestion_control(&dctcp);
- if (ret)
- return ret;
- register_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_dctcp_kfunc_btf_set);
- return 0;
+ return tcp_register_congestion_control(&dctcp);
}
static void __exit dctcp_unregister(void)
{
- unregister_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_dctcp_kfunc_btf_set);
tcp_unregister_congestion_control(&dctcp);
}
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 5d52ea2768df..13b2af3fa17a 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -89,26 +89,17 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
.write = bpf_testmod_test_write,
};
-BTF_SET_START(bpf_testmod_kfunc_ids)
+BTF_KFUNC_SET_START(tc, check, bpf_testmod)
BTF_ID(func, bpf_testmod_test_mod_kfunc)
-BTF_SET_END(bpf_testmod_kfunc_ids)
-
-static DEFINE_KFUNC_BTF_ID_SET(&bpf_testmod_kfunc_ids, bpf_testmod_kfunc_btf_set);
+BTF_KFUNC_SET_END(tc, check, bpf_testmod)
static int bpf_testmod_init(void)
{
- int ret;
-
- ret = sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
- if (ret)
- return ret;
- register_kfunc_btf_id_set(&prog_test_kfunc_list, &bpf_testmod_kfunc_btf_set);
- return 0;
+ return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
}
static void bpf_testmod_exit(void)
{
- unregister_kfunc_btf_id_set(&prog_test_kfunc_list, &bpf_testmod_kfunc_btf_set);
return sysfs_remove_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH bpf-next v5 4/9] bpf: Introduce mem, size argument pair support for kfunc
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
` (2 preceding siblings ...)
2021-12-30 2:36 ` [PATCH bpf-next v5 3/9] bpf: Remove check_kfunc_call callback and old kfunc BTF ID API Kumar Kartikeya Dwivedi
@ 2021-12-30 2:37 ` Kumar Kartikeya Dwivedi
2021-12-30 2:37 ` [PATCH bpf-next v5 5/9] bpf: Add reference tracking support to kfunc Kumar Kartikeya Dwivedi
` (5 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:37 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
BPF helpers can associate two adjacent arguments together to pass memory
of certain size, using ARG_PTR_TO_MEM and ARG_CONST_SIZE arguments.
Since we don't use bpf_func_proto for kfunc, we need to leverage BTF to
implement similar support.
The ARG_CONST_SIZE processing for helpers is refactored into a common
check_mem_size_reg helper that is shared with kfunc as well. kfunc
ptr_to_mem support follows logic similar to global functions, where
verification is done as if pointer is not null, even when it may be
null.
This leads to a simple to follow rule for writing kfunc: always check
the argument pointer for NULL, except when it is PTR_TO_CTX. Also, the
PTR_TO_CTX case is also only safe when the helper expecting pointer to
program ctx is not exposed to other programs where same struct is not
ctx type. In that case, the type check will fall through to other cases
and would permit passing other types of pointers, possibly NULL at
runtime.
Currently, we require the size argument to be suffixed with "__sz" in
the parameter name. This information is then recorded in kernel BTF and
verified during function argument checking. In the future we can use BTF
tagging instead, and modify the kernel function definitions. This will
be a purely kernel-side change.
This allows us to have some form of backwards compatibility for
structures that are passed in to the kernel function with their size,
and allow variable length structures to be passed in if they are
accompanied by a size parameter.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/bpf_verifier.h | 2 +
kernel/bpf/btf.c | 46 ++++++++++++-
kernel/bpf/verifier.c | 124 ++++++++++++++++++++++-------------
3 files changed, 124 insertions(+), 48 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 143401d4c9d9..857fd687bdc2 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -521,6 +521,8 @@ bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt);
int check_ctx_reg(struct bpf_verifier_env *env,
const struct bpf_reg_state *reg, int regno);
+int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
+ u32 regno);
int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
u32 regno, u32 mem_size);
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index f5b3049a56e0..fc1142a044c4 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5887,6 +5887,30 @@ static bool __btf_type_is_scalar_struct(struct bpf_verifier_log *log,
return true;
}
+static bool is_kfunc_arg_mem_size(const struct btf *btf,
+ const struct btf_param *arg,
+ const struct bpf_reg_state *reg)
+{
+ int len, sfx_len = sizeof("__sz") - 1;
+ const struct btf_type *t;
+ const char *param_name;
+
+ t = btf_type_skip_modifiers(btf, arg->type, NULL);
+ if (!btf_type_is_scalar(t) || reg->type != SCALAR_VALUE)
+ return false;
+
+ /* In the future, this can be ported to use BTF tagging */
+ param_name = btf_name_by_offset(btf, arg->name_off);
+ len = strlen(param_name);
+ if (len < sfx_len)
+ return false;
+ param_name += len - sfx_len;
+ if (strncmp(param_name, "__sz", sfx_len))
+ return false;
+
+ return true;
+}
+
static int btf_check_func_arg_match(struct bpf_verifier_env *env,
const struct btf *btf, u32 func_id,
struct bpf_reg_state *regs,
@@ -5998,17 +6022,33 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
u32 type_size;
if (is_kfunc) {
+ bool arg_mem_size = i + 1 < nargs && is_kfunc_arg_mem_size(btf, &args[i + 1], ®s[regno + 1]);
+
/* Permit pointer to mem, but only when argument
* type is pointer to scalar, or struct composed
* (recursively) of scalars.
+ * When arg_mem_size is true, the pointer can be
+ * void *.
*/
if (!btf_type_is_scalar(ref_t) &&
- !__btf_type_is_scalar_struct(log, btf, ref_t, 0)) {
+ !__btf_type_is_scalar_struct(log, btf, ref_t, 0) &&
+ (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) {
bpf_log(log,
- "arg#%d pointer type %s %s must point to scalar or struct with scalar\n",
- i, btf_type_str(ref_t), ref_tname);
+ "arg#%d pointer type %s %s must point to %sscalar, or struct with scalar\n",
+ i, btf_type_str(ref_t), ref_tname, arg_mem_size ? "void, " : "");
return -EINVAL;
}
+
+ /* Check for mem, len pair */
+ if (arg_mem_size) {
+ if (check_kfunc_mem_size_reg(env, ®s[regno + 1], regno + 1)) {
+ bpf_log(log, "arg#%d arg#%d memory, len pair leads to invalid memory access\n",
+ i, i + 1);
+ return -EINVAL;
+ }
+ i++;
+ continue;
+ }
}
resolve_ret = btf_resolve_size(btf, ref_t, &type_size);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e9503192101f..7ec13d146b05 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4849,6 +4849,62 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
}
}
+static int check_mem_size_reg(struct bpf_verifier_env *env,
+ struct bpf_reg_state *reg, u32 regno,
+ bool zero_size_allowed,
+ struct bpf_call_arg_meta *meta)
+{
+ int err;
+
+ /* This is used to refine r0 return value bounds for helpers
+ * that enforce this value as an upper bound on return values.
+ * See do_refine_retval_range() for helpers that can refine
+ * the return value. C type of helper is u32 so we pull register
+ * bound from umax_value however, if negative verifier errors
+ * out. Only upper bounds can be learned because retval is an
+ * int type and negative retvals are allowed.
+ */
+ if (meta)
+ meta->msize_max_value = reg->umax_value;
+
+ /* The register is SCALAR_VALUE; the access check
+ * happens using its boundaries.
+ */
+ if (!tnum_is_const(reg->var_off))
+ /* For unprivileged variable accesses, disable raw
+ * mode so that the program is required to
+ * initialize all the memory that the helper could
+ * just partially fill up.
+ */
+ meta = NULL;
+
+ if (reg->smin_value < 0) {
+ verbose(env, "R%d min value is negative, either use unsigned or 'var &= const'\n",
+ regno);
+ return -EACCES;
+ }
+
+ if (reg->umin_value == 0) {
+ err = check_helper_mem_access(env, regno - 1, 0,
+ zero_size_allowed,
+ meta);
+ if (err)
+ return err;
+ }
+
+ if (reg->umax_value >= BPF_MAX_VAR_SIZ) {
+ verbose(env, "R%d unbounded memory access, use 'var &= const' or 'if (var < const)'\n",
+ regno);
+ return -EACCES;
+ }
+ err = check_helper_mem_access(env, regno - 1,
+ reg->umax_value,
+ zero_size_allowed, meta);
+ if (!err)
+ err = mark_chain_precision(env, regno);
+ return err;
+}
+
int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
u32 regno, u32 mem_size)
{
@@ -4872,6 +4928,28 @@ int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
return check_helper_mem_access(env, regno, mem_size, true, NULL);
}
+int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
+ u32 regno)
+{
+ struct bpf_reg_state *mem_reg = &cur_regs(env)[regno - 1];
+ bool may_be_null = type_may_be_null(mem_reg->type);
+ struct bpf_reg_state saved_reg;
+ int err;
+
+ WARN_ON_ONCE(regno < BPF_REG_2 || regno > BPF_REG_5);
+
+ if (may_be_null) {
+ saved_reg = *mem_reg;
+ mark_ptr_not_null_reg(mem_reg);
+ }
+
+ err = check_mem_size_reg(env, reg, regno, true, NULL);
+
+ if (may_be_null)
+ *mem_reg = saved_reg;
+ return err;
+}
+
/* Implementation details:
* bpf_map_lookup returns PTR_TO_MAP_VALUE_OR_NULL
* Two bpf_map_lookups (even with the same key) will have different reg->id.
@@ -5393,51 +5471,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
} else if (arg_type_is_mem_size(arg_type)) {
bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);
- /* This is used to refine r0 return value bounds for helpers
- * that enforce this value as an upper bound on return values.
- * See do_refine_retval_range() for helpers that can refine
- * the return value. C type of helper is u32 so we pull register
- * bound from umax_value however, if negative verifier errors
- * out. Only upper bounds can be learned because retval is an
- * int type and negative retvals are allowed.
- */
- meta->msize_max_value = reg->umax_value;
-
- /* The register is SCALAR_VALUE; the access check
- * happens using its boundaries.
- */
- if (!tnum_is_const(reg->var_off))
- /* For unprivileged variable accesses, disable raw
- * mode so that the program is required to
- * initialize all the memory that the helper could
- * just partially fill up.
- */
- meta = NULL;
-
- if (reg->smin_value < 0) {
- verbose(env, "R%d min value is negative, either use unsigned or 'var &= const'\n",
- regno);
- return -EACCES;
- }
-
- if (reg->umin_value == 0) {
- err = check_helper_mem_access(env, regno - 1, 0,
- zero_size_allowed,
- meta);
- if (err)
- return err;
- }
-
- if (reg->umax_value >= BPF_MAX_VAR_SIZ) {
- verbose(env, "R%d unbounded memory access, use 'var &= const' or 'if (var < const)'\n",
- regno);
- return -EACCES;
- }
- err = check_helper_mem_access(env, regno - 1,
- reg->umax_value,
- zero_size_allowed, meta);
- if (!err)
- err = mark_chain_precision(env, regno);
+ err = check_mem_size_reg(env, reg, regno, zero_size_allowed, meta);
} else if (arg_type_is_alloc_size(arg_type)) {
if (!tnum_is_const(reg->var_off)) {
verbose(env, "R%d is not a known constant'\n",
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH bpf-next v5 5/9] bpf: Add reference tracking support to kfunc
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
` (3 preceding siblings ...)
2021-12-30 2:37 ` [PATCH bpf-next v5 4/9] bpf: Introduce mem, size argument pair support for kfunc Kumar Kartikeya Dwivedi
@ 2021-12-30 2:37 ` Kumar Kartikeya Dwivedi
2021-12-30 2:37 ` [PATCH bpf-next v5 6/9] net/netfilter: Add unstable CT lookup helpers for XDP and TC-BPF Kumar Kartikeya Dwivedi
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:37 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
This patch adds verifier support for PTR_TO_BTF_ID return type of kfunc
to be a reference, by reusing acquire_reference_state/release_reference
support for existing in-kernel bpf helpers.
We make use of the three kfunc types:
- BTF_KFUNC_TYPE_ACQUIRE
Return true if kfunc_btf_id is an acquire kfunc. This will
acquire_reference_state for the returned PTR_TO_BTF_ID (this is the
only allow return value). Note that acquire kfunc must always return a
PTR_TO_BTF_ID{_OR_NULL}, otherwise the program is rejected.
- BTF_KFUNC_TYPE_RELEASE
Return true if kfunc_btf_id is a release kfunc. This will release the
reference to the passed in PTR_TO_BTF_ID which has a reference state
(from earlier acquire kfunc).
The btf_check_func_arg_match returns the regno (of argument register,
hence > 0) if the kfunc is a release kfunc, and a proper referenced
PTR_TO_BTF_ID is being passed to it.
This is similar to how helper call check uses bpf_call_arg_meta to
store the ref_obj_id that is later used to release the reference.
Similar to in-kernel helper, we only allow passing one referenced
PTR_TO_BTF_ID as an argument. It can also be passed in to normal
kfunc, but in case of release kfunc there must always be one
PTR_TO_BTF_ID argument that is referenced.
- BTF_KFUNC_TYPE_RET_NULL
For kfunc returning PTR_TO_BTF_ID, tells if it can be NULL, hence
force caller to mark the pointer not null (using check) before
accessing it. Note that taking into account the case fixed by commit
93c230e3f5bd ("bpf: Enforce id generation for all may-be-null register type")
we assign a non-zero id for mark_ptr_or_null_reg logic. Later, if more
return types are supported by kfunc, which have a _OR_NULL variant, it
might be better to move this id generation under a common
reg_type_may_be_null check, similar to the case in the commit.
Referenced PTR_TO_BTF_ID is currently only limited to kfunc, but can be
extended in the future to other BPF helpers as well. For now, we can
rely on the btf_struct_ids_match check to ensure we get the pointer to
the expected struct type. In the future, care needs to be taken to avoid
ambiguity for reference PTR_TO_BTF_ID passed to release function, in
case multiple candidates can release same BTF ID.
e.g. there might be two release kfuncs (or kfunc and helper):
foo(struct abc *p);
bar(struct abc *p);
... such that both release a PTR_TO_BTF_ID with btf_id of struct abc. In
this case we would need to track the acquire function corresponding to
the release function to avoid type confusion, and store this information
in the register state so that an incorrect program can be rejected. This
is not a problem right now, hence it is left as an exercise for the
future patch introducing such a case in the kernel.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/bpf_verifier.h | 5 ++++
kernel/bpf/btf.c | 32 ++++++++++++++++++++--
kernel/bpf/verifier.c | 52 +++++++++++++++++++++++++++++-------
3 files changed, 77 insertions(+), 12 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 857fd687bdc2..ac4797155412 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -566,4 +566,9 @@ static inline u32 type_flag(u32 type)
return type & ~BPF_BASE_TYPE_MASK;
}
+static inline enum bpf_prog_type resolve_prog_type(struct bpf_prog *prog)
+{
+ return prog->aux->dst_prog ? prog->aux->dst_prog->type : prog->type;
+}
+
#endif /* _LINUX_BPF_VERIFIER_H */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index fc1142a044c4..ba7b607cf6e4 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5917,11 +5917,13 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
bool ptr_to_mem_ok)
{
struct bpf_verifier_log *log = &env->log;
+ u32 i, nargs, ref_id, ref_obj_id = 0;
bool is_kfunc = btf_is_kernel(btf);
const char *func_name, *ref_tname;
const struct btf_type *t, *ref_t;
const struct btf_param *args;
- u32 i, nargs, ref_id;
+ int ref_regno = 0;
+ bool rel = false;
t = btf_type_by_id(btf, func_id);
if (!t || !btf_type_is_func(t)) {
@@ -5999,6 +6001,16 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
if (reg->type == PTR_TO_BTF_ID) {
reg_btf = reg->btf;
reg_ref_id = reg->btf_id;
+ /* Ensure only one argument is referenced PTR_TO_BTF_ID */
+ if (reg->ref_obj_id) {
+ if (ref_obj_id) {
+ bpf_log(log, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n",
+ regno, reg->ref_obj_id, ref_obj_id);
+ return -EFAULT;
+ }
+ ref_regno = regno;
+ ref_obj_id = reg->ref_obj_id;
+ }
} else {
reg_btf = btf_vmlinux;
reg_ref_id = *reg2btf_ids[reg->type];
@@ -6069,7 +6081,23 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
}
}
- return 0;
+ /* Either both are set, or neither */
+ WARN_ON_ONCE((ref_obj_id && !ref_regno) || (!ref_obj_id && ref_regno));
+ if (is_kfunc) {
+ rel = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
+ BTF_KFUNC_TYPE_RELEASE, func_id);
+ /* We already made sure ref_obj_id is set only for one argument */
+ if (rel && !ref_obj_id) {
+ bpf_log(log, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n",
+ func_name);
+ return -EINVAL;
+ }
+ /* Allow (!rel && ref_obj_id), so that passing such referenced PTR_TO_BTF_ID to
+ * other kfuncs works
+ */
+ }
+ /* returns argument register number > 0 in case of reference release kfunc */
+ return rel ? ref_regno : 0;
}
/* Compare BTF of a function with given bpf_reg_state.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7ec13d146b05..a2c53e126a0f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -452,7 +452,8 @@ static bool reg_type_may_be_refcounted_or_null(enum bpf_reg_type type)
{
return base_type(type) == PTR_TO_SOCKET ||
base_type(type) == PTR_TO_TCP_SOCK ||
- base_type(type) == PTR_TO_MEM;
+ base_type(type) == PTR_TO_MEM ||
+ base_type(type) == PTR_TO_BTF_ID;
}
static bool type_is_rdonly_mem(u32 type)
@@ -3491,11 +3492,6 @@ static int check_map_access(struct bpf_verifier_env *env, u32 regno,
#define MAX_PACKET_OFF 0xffff
-static enum bpf_prog_type resolve_prog_type(struct bpf_prog *prog)
-{
- return prog->aux->dst_prog ? prog->aux->dst_prog->type : prog->type;
-}
-
static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
const struct bpf_call_arg_meta *meta,
enum bpf_access_type t)
@@ -6829,15 +6825,17 @@ static void mark_btf_func_reg_size(struct bpf_verifier_env *env, u32 regno,
}
}
-static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
+static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
+ int *insn_idx_p)
{
const struct btf_type *t, *func, *func_proto, *ptr_type;
struct bpf_reg_state *regs = cur_regs(env);
const char *func_name, *ptr_type_name;
u32 i, nargs, func_id, ptr_type_id;
+ int err, insn_idx = *insn_idx_p;
const struct btf_param *args;
struct btf *desc_btf;
- int err;
+ bool acq;
/* skip for now, but return error when we find this in fixup_kfunc_call */
if (!insn->imm)
@@ -6859,16 +6857,36 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
return -EACCES;
}
+ acq = btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog),
+ BTF_KFUNC_TYPE_ACQUIRE, func_id);
+
/* Check the arguments */
err = btf_check_kfunc_arg_match(env, desc_btf, func_id, regs);
- if (err)
+ if (err < 0)
return err;
+ /* In case of release function, we get register number of refcounted
+ * PTR_TO_BTF_ID back from btf_check_kfunc_arg_match, do the release now
+ */
+ if (err) {
+ err = release_reference(env, regs[err].ref_obj_id);
+ if (err) {
+ verbose(env, "kfunc %s#%d reference has not been acquired before\n",
+ func_name, func_id);
+ return err;
+ }
+ }
for (i = 0; i < CALLER_SAVED_REGS; i++)
mark_reg_not_init(env, regs, caller_saved[i]);
/* Check return type */
t = btf_type_skip_modifiers(desc_btf, func_proto->type, NULL);
+
+ if (acq && !btf_type_is_ptr(t)) {
+ verbose(env, "acquire kernel function does not return PTR_TO_BTF_ID\n");
+ return -EINVAL;
+ }
+
if (btf_type_is_scalar(t)) {
mark_reg_unknown(env, regs, BPF_REG_0);
mark_btf_func_reg_size(env, BPF_REG_0, t->size);
@@ -6887,7 +6905,21 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
regs[BPF_REG_0].btf = desc_btf;
regs[BPF_REG_0].type = PTR_TO_BTF_ID;
regs[BPF_REG_0].btf_id = ptr_type_id;
+ if (btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog),
+ BTF_KFUNC_TYPE_RET_NULL, func_id)) {
+ regs[BPF_REG_0].type |= PTR_MAYBE_NULL;
+ /* For mark_ptr_or_null_reg, see 93c230e3f5bd6 */
+ regs[BPF_REG_0].id = ++env->id_gen;
+ }
mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *));
+ if (acq) {
+ int id = acquire_reference_state(env, insn_idx);
+
+ if (id < 0)
+ return id;
+ regs[BPF_REG_0].id = id;
+ regs[BPF_REG_0].ref_obj_id = id;
+ }
} /* else { add_kfunc_call() ensures it is btf_type_is_void(t) } */
nargs = btf_type_vlen(func_proto);
@@ -11528,7 +11560,7 @@ static int do_check(struct bpf_verifier_env *env)
if (insn->src_reg == BPF_PSEUDO_CALL)
err = check_func_call(env, insn, &env->insn_idx);
else if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL)
- err = check_kfunc_call(env, insn);
+ err = check_kfunc_call(env, insn, &env->insn_idx);
else
err = check_helper_call(env, insn, &env->insn_idx);
if (err)
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH bpf-next v5 6/9] net/netfilter: Add unstable CT lookup helpers for XDP and TC-BPF
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
` (4 preceding siblings ...)
2021-12-30 2:37 ` [PATCH bpf-next v5 5/9] bpf: Add reference tracking support to kfunc Kumar Kartikeya Dwivedi
@ 2021-12-30 2:37 ` Kumar Kartikeya Dwivedi
2021-12-30 2:37 ` [PATCH bpf-next v5 7/9] selftests/bpf: Add test for unstable CT lookup API Kumar Kartikeya Dwivedi
` (3 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:37 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
This change adds conntrack lookup helpers using the unstable kfunc call
interface for the XDP and TC-BPF hooks. The primary usecase is
implementing a synproxy in XDP, see Maxim's patchset [0].
Export get_net_ns_by_id as nf_conntrack_bpf.c needs to call it.
This object is only built when CONFIG_DEBUG_INFO_BTF_MODULES is enabled.
[0]: https://lore.kernel.org/bpf/20211019144655.3483197-1-maximmi@nvidia.com
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
net/core/net_namespace.c | 1 +
net/netfilter/Makefile | 5 +
net/netfilter/nf_conntrack_bpf.c | 253 +++++++++++++++++++++++++++++++
3 files changed, 259 insertions(+)
create mode 100644 net/netfilter/nf_conntrack_bpf.c
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 9b7171c40434..3b471781327f 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -299,6 +299,7 @@ struct net *get_net_ns_by_id(const struct net *net, int id)
return peer;
}
+EXPORT_SYMBOL_GPL(get_net_ns_by_id);
/*
* setup_net runs the initializers for the network namespace object.
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index aab20e575ecd..39338c957d77 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -14,6 +14,11 @@ nf_conntrack-$(CONFIG_NF_CONNTRACK_LABELS) += nf_conntrack_labels.o
nf_conntrack-$(CONFIG_NF_CT_PROTO_DCCP) += nf_conntrack_proto_dccp.o
nf_conntrack-$(CONFIG_NF_CT_PROTO_SCTP) += nf_conntrack_proto_sctp.o
nf_conntrack-$(CONFIG_NF_CT_PROTO_GRE) += nf_conntrack_proto_gre.o
+ifeq ($(CONFIG_NF_CONNTRACK),m)
+nf_conntrack-$(CONFIG_DEBUG_INFO_BTF_MODULES) += nf_conntrack_bpf.o
+else ifeq ($(CONFIG_NF_CONNTRACK),y)
+nf_conntrack-$(CONFIG_DEBUG_INFO_BTF) += nf_conntrack_bpf.o
+endif
obj-$(CONFIG_NETFILTER) = netfilter.o
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
new file mode 100644
index 000000000000..878d5bf947e1
--- /dev/null
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Unstable Conntrack Helpers for XDP and TC-BPF hook
+ *
+ * These are called from the XDP and SCHED_CLS BPF programs. Note that it is
+ * allowed to break compatibility for these functions since the interface they
+ * are exposed through to BPF programs is explicitly unstable.
+ */
+
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/types.h>
+#include <linux/btf_ids.h>
+#include <linux/net_namespace.h>
+#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_core.h>
+
+/* bpf_ct_opts - Options for CT lookup helpers
+ *
+ * Members:
+ * @netns_id - Specify the network namespace for lookup
+ * Values:
+ * BPF_F_CURRENT_NETNS (-1)
+ * Use namespace associated with ctx (xdp_md, __sk_buff)
+ * [0, S32_MAX]
+ * Network Namespace ID
+ * @error - Out parameter, set for any errors encountered
+ * Values:
+ * -EINVAL - Passed NULL for bpf_tuple pointer
+ * -EINVAL - opts->reserved is not 0
+ * -EINVAL - netns_id is less than -1
+ * -EINVAL - opts__sz isn't NF_BPF_CT_OPTS_SZ (12)
+ * -EPROTO - l4proto isn't one of IPPROTO_TCP or IPPROTO_UDP
+ * -ENONET - No network namespace found for netns_id
+ * -ENOENT - Conntrack lookup could not find entry for tuple
+ * -EAFNOSUPPORT - tuple__sz isn't one of sizeof(tuple->ipv4)
+ * or sizeof(tuple->ipv6)
+ * @l4proto - Layer 4 protocol
+ * Values:
+ * IPPROTO_TCP, IPPROTO_UDP
+ * @reserved - Reserved member, will be reused for more options in future
+ * Values:
+ * 0
+ */
+struct bpf_ct_opts {
+ s32 netns_id;
+ s32 error;
+ u8 l4proto;
+ u8 reserved[3];
+};
+
+enum {
+ NF_BPF_CT_OPTS_SZ = 12,
+};
+
+static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
+ struct bpf_sock_tuple *bpf_tuple,
+ u32 tuple_len, u8 protonum,
+ s32 netns_id)
+{
+ struct nf_conntrack_tuple_hash *hash;
+ struct nf_conntrack_tuple tuple;
+
+ if (unlikely(protonum != IPPROTO_TCP && protonum != IPPROTO_UDP))
+ return ERR_PTR(-EPROTO);
+ if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
+ return ERR_PTR(-EINVAL);
+
+ memset(&tuple, 0, sizeof(tuple));
+ switch (tuple_len) {
+ case sizeof(bpf_tuple->ipv4):
+ tuple.src.l3num = AF_INET;
+ tuple.src.u3.ip = bpf_tuple->ipv4.saddr;
+ tuple.src.u.tcp.port = bpf_tuple->ipv4.sport;
+ tuple.dst.u3.ip = bpf_tuple->ipv4.daddr;
+ tuple.dst.u.tcp.port = bpf_tuple->ipv4.dport;
+ break;
+ case sizeof(bpf_tuple->ipv6):
+ tuple.src.l3num = AF_INET6;
+ memcpy(tuple.src.u3.ip6, bpf_tuple->ipv6.saddr, sizeof(bpf_tuple->ipv6.saddr));
+ tuple.src.u.tcp.port = bpf_tuple->ipv6.sport;
+ memcpy(tuple.dst.u3.ip6, bpf_tuple->ipv6.daddr, sizeof(bpf_tuple->ipv6.daddr));
+ tuple.dst.u.tcp.port = bpf_tuple->ipv6.dport;
+ break;
+ default:
+ return ERR_PTR(-EAFNOSUPPORT);
+ }
+
+ tuple.dst.protonum = protonum;
+
+ if (netns_id >= 0) {
+ net = get_net_ns_by_id(net, netns_id);
+ if (unlikely(!net))
+ return ERR_PTR(-ENONET);
+ }
+
+ hash = nf_conntrack_find_get(net, &nf_ct_zone_dflt, &tuple);
+ if (netns_id >= 0)
+ put_net(net);
+ if (!hash)
+ return ERR_PTR(-ENOENT);
+ return nf_ct_tuplehash_to_ctrack(hash);
+}
+
+__diag_push();
+__diag_ignore(GCC, 8, "-Wmissing-prototypes",
+ "Global functions as their definitions will be in nf_conntrack BTF");
+
+/* bpf_xdp_ct_lookup - Lookup CT entry for the given tuple, and acquire a
+ * reference to it
+ *
+ * Parameters:
+ * @xdp_ctx - Pointer to ctx (xdp_md) in XDP program
+ * Cannot be NULL
+ * @bpf_tuple - Pointer to memory representing the tuple to look up
+ * Cannot be NULL
+ * @tuple__sz - Length of the tuple structure
+ * Must be one of sizeof(bpf_tuple->ipv4) or
+ * sizeof(bpf_tuple->ipv6)
+ * @opts - Additional options for lookup (documented above)
+ * Cannot be NULL
+ * @opts__sz - Length of the bpf_ct_opts structure
+ * Must be NF_BPF_CT_OPTS_SZ (12)
+ */
+struct nf_conn *
+bpf_xdp_ct_lookup(struct xdp_md *xdp_ctx, struct bpf_sock_tuple *bpf_tuple,
+ u32 tuple__sz, struct bpf_ct_opts *opts, u32 opts__sz)
+{
+ struct xdp_buff *ctx = (struct xdp_buff *)xdp_ctx;
+ struct net *caller_net;
+ struct nf_conn *nfct;
+
+ BUILD_BUG_ON(sizeof(struct bpf_ct_opts) != NF_BPF_CT_OPTS_SZ);
+
+ if (!opts)
+ return NULL;
+ if (!bpf_tuple || opts->reserved[0] || opts->reserved[1] ||
+ opts->reserved[2] || opts__sz != NF_BPF_CT_OPTS_SZ) {
+ opts->error = -EINVAL;
+ return NULL;
+ }
+ caller_net = dev_net(ctx->rxq->dev);
+ nfct = __bpf_nf_ct_lookup(caller_net, bpf_tuple, tuple__sz, opts->l4proto,
+ opts->netns_id);
+ if (IS_ERR(nfct)) {
+ opts->error = PTR_ERR(nfct);
+ return NULL;
+ }
+ return nfct;
+}
+
+/* bpf_skb_ct_lookup - Lookup CT entry for the given tuple, and acquire a
+ * reference to it
+ *
+ * Parameters:
+ * @skb_ctx - Pointer to ctx (__sk_buff) in TC program
+ * Cannot be NULL
+ * @bpf_tuple - Pointer to memory representing the tuple to look up
+ * Cannot be NULL
+ * @tuple__sz - Length of the tuple structure
+ * Must be one of sizeof(bpf_tuple->ipv4) or
+ * sizeof(bpf_tuple->ipv6)
+ * @opts - Additional options for lookup (documented above)
+ * Cannot be NULL
+ * @opts__sz - Length of the bpf_ct_opts structure
+ * Must be NF_BPF_CT_OPTS_SZ (12)
+ */
+struct nf_conn *
+bpf_skb_ct_lookup(struct __sk_buff *skb_ctx, struct bpf_sock_tuple *bpf_tuple,
+ u32 tuple__sz, struct bpf_ct_opts *opts, u32 opts__sz)
+{
+ struct sk_buff *skb = (struct sk_buff *)skb_ctx;
+ struct net *caller_net;
+ struct nf_conn *nfct;
+
+ BUILD_BUG_ON(sizeof(struct bpf_ct_opts) != NF_BPF_CT_OPTS_SZ);
+
+ if (!opts)
+ return NULL;
+ if (!bpf_tuple || opts->reserved[0] || opts->reserved[1] ||
+ opts->reserved[2] || opts__sz != NF_BPF_CT_OPTS_SZ) {
+ opts->error = -EINVAL;
+ return NULL;
+ }
+ caller_net = skb->dev ? dev_net(skb->dev) : sock_net(skb->sk);
+ nfct = __bpf_nf_ct_lookup(caller_net, bpf_tuple, tuple__sz, opts->l4proto,
+ opts->netns_id);
+ if (IS_ERR(nfct)) {
+ opts->error = PTR_ERR(nfct);
+ return NULL;
+ }
+ return nfct;
+}
+
+/* bpf_ct_release - Release acquired nf_conn object
+ *
+ * This must be invoked for referenced PTR_TO_BTF_ID, and the verifier rejects
+ * the program if any references remain in the program in all of the explored
+ * states.
+ *
+ * Parameters:
+ * @nf_conn - Pointer to referenced nf_conn object, obtained using
+ * bpf_xdp_ct_lookup or bpf_skb_ct_lookup.
+ */
+void bpf_ct_release(struct nf_conn *nfct)
+{
+ if (!nfct)
+ return;
+ nf_ct_put(nfct);
+}
+
+__diag_pop()
+
+/* XDP hook allowed kfuncs */
+BTF_KFUNC_SET_START(xdp, check, nf_conntrack)
+BTF_ID(func, bpf_xdp_ct_lookup)
+BTF_ID(func, bpf_ct_release)
+BTF_KFUNC_SET_END(xdp, check, nf_conntrack)
+
+/* XDP hook acquire kfuncs */
+BTF_KFUNC_SET_START(xdp, acquire, nf_conntrack)
+BTF_ID(func, bpf_xdp_ct_lookup)
+BTF_KFUNC_SET_END(xdp, acquire, nf_conntrack)
+
+/* XDP hook release kfuncs */
+BTF_KFUNC_SET_START(xdp, release, nf_conntrack)
+BTF_ID(func, bpf_ct_release)
+BTF_KFUNC_SET_END(xdp, release, nf_conntrack)
+
+/* XDP hook 'ret type NULL' kfuncs */
+BTF_KFUNC_SET_START(xdp, ret_null, nf_conntrack)
+BTF_ID(func, bpf_xdp_ct_lookup)
+BTF_KFUNC_SET_END(xdp, ret_null, nf_conntrack)
+
+/* TC hook allowed kfuncs */
+BTF_KFUNC_SET_START(tc, check, nf_conntrack)
+BTF_ID(func, bpf_skb_ct_lookup)
+BTF_ID(func, bpf_ct_release)
+BTF_KFUNC_SET_END(tc, check, nf_conntrack)
+
+/* TC hook acquire kfuncs */
+BTF_KFUNC_SET_START(tc, acquire, nf_conntrack)
+BTF_ID(func, bpf_skb_ct_lookup)
+BTF_KFUNC_SET_END(tc, acquire, nf_conntrack)
+
+/* TC hook release kfuncs */
+BTF_KFUNC_SET_START(tc, release, nf_conntrack)
+BTF_ID(func, bpf_ct_release)
+BTF_KFUNC_SET_END(tc, release, nf_conntrack)
+
+/* TC hook 'ret type NULL' kfuncs */
+BTF_KFUNC_SET_START(tc, ret_null, nf_conntrack)
+BTF_ID(func, bpf_skb_ct_lookup)
+BTF_KFUNC_SET_END(tc, ret_null, nf_conntrack)
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH bpf-next v5 7/9] selftests/bpf: Add test for unstable CT lookup API
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
` (5 preceding siblings ...)
2021-12-30 2:37 ` [PATCH bpf-next v5 6/9] net/netfilter: Add unstable CT lookup helpers for XDP and TC-BPF Kumar Kartikeya Dwivedi
@ 2021-12-30 2:37 ` Kumar Kartikeya Dwivedi
2021-12-30 2:37 ` [PATCH bpf-next v5 8/9] selftests/bpf: Add test_verifier support to fixup kfunc call insns Kumar Kartikeya Dwivedi
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:37 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
This tests that we return errors as documented, and also that the kfunc
calls work from both XDP and TC hooks.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
tools/testing/selftests/bpf/config | 4 +
.../testing/selftests/bpf/prog_tests/bpf_nf.c | 48 ++++++++
.../testing/selftests/bpf/progs/test_bpf_nf.c | 105 ++++++++++++++++++
3 files changed, 157 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/bpf_nf.c
create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_nf.c
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index f6287132fa89..32d80e77e910 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -48,3 +48,7 @@ CONFIG_IMA_READ_POLICY=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_FUNCTION_TRACER=y
CONFIG_DYNAMIC_FTRACE=y
+CONFIG_NETFILTER=y
+CONFIG_NF_DEFRAG_IPV4=y
+CONFIG_NF_DEFRAG_IPV6=y
+CONFIG_NF_CONNTRACK=y
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
new file mode 100644
index 000000000000..e3166a81e989
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include "test_bpf_nf.skel.h"
+
+enum {
+ TEST_XDP,
+ TEST_TC_BPF,
+};
+
+void test_bpf_nf_ct(int mode)
+{
+ struct test_bpf_nf *skel;
+ int prog_fd, err, retval;
+
+ skel = test_bpf_nf__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "test_bpf_nf__open_and_load"))
+ return;
+
+ if (mode == TEST_XDP)
+ prog_fd = bpf_program__fd(skel->progs.nf_xdp_ct_test);
+ else
+ prog_fd = bpf_program__fd(skel->progs.nf_skb_ct_test);
+
+ err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4), NULL, NULL,
+ (__u32 *)&retval, NULL);
+ if (!ASSERT_OK(err, "bpf_prog_test_run"))
+ goto end;
+
+ ASSERT_EQ(skel->bss->test_einval_bpf_tuple, -EINVAL, "Test EINVAL for NULL bpf_tuple");
+ ASSERT_EQ(skel->bss->test_einval_reserved, -EINVAL, "Test EINVAL for reserved not set to 0");
+ ASSERT_EQ(skel->bss->test_einval_netns_id, -EINVAL, "Test EINVAL for netns_id < -1");
+ ASSERT_EQ(skel->bss->test_einval_len_opts, -EINVAL, "Test EINVAL for len__opts != NF_BPF_CT_OPTS_SZ");
+ ASSERT_EQ(skel->bss->test_eproto_l4proto, -EPROTO, "Test EPROTO for l4proto != TCP or UDP");
+ ASSERT_EQ(skel->bss->test_enonet_netns_id, -ENONET, "Test ENONET for bad but valid netns_id");
+ ASSERT_EQ(skel->bss->test_enoent_lookup, -ENOENT, "Test ENOENT for failed lookup");
+ ASSERT_EQ(skel->bss->test_eafnosupport, -EAFNOSUPPORT, "Test EAFNOSUPPORT for invalid len__tuple");
+end:
+ test_bpf_nf__destroy(skel);
+}
+
+void test_bpf_nf(void)
+{
+ if (test__start_subtest("xdp-ct"))
+ test_bpf_nf_ct(TEST_XDP);
+ if (test__start_subtest("tc-bpf-ct"))
+ test_bpf_nf_ct(TEST_TC_BPF);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
new file mode 100644
index 000000000000..d6d4002ad69c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+#define EAFNOSUPPORT 97
+#define EPROTO 71
+#define ENONET 64
+#define EINVAL 22
+#define ENOENT 2
+
+int test_einval_bpf_tuple = 0;
+int test_einval_reserved = 0;
+int test_einval_netns_id = 0;
+int test_einval_len_opts = 0;
+int test_eproto_l4proto = 0;
+int test_enonet_netns_id = 0;
+int test_enoent_lookup = 0;
+int test_eafnosupport = 0;
+
+struct nf_conn *bpf_xdp_ct_lookup(struct xdp_md *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts *, u32) __ksym;
+struct nf_conn *bpf_skb_ct_lookup(struct __sk_buff *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts *, u32) __ksym;
+void bpf_ct_release(struct nf_conn *) __ksym;
+
+#define nf_ct_test(func, ctx) \
+ ({ \
+ struct bpf_ct_opts opts_def = { .l4proto = IPPROTO_TCP, \
+ .netns_id = -1 }; \
+ struct bpf_sock_tuple bpf_tuple; \
+ struct nf_conn *ct; \
+ __builtin_memset(&bpf_tuple, 0, sizeof(bpf_tuple.ipv4)); \
+ ct = func(ctx, NULL, 0, &opts_def, sizeof(opts_def)); \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_einval_bpf_tuple = opts_def.error; \
+ opts_def.reserved[0] = 1; \
+ ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def, \
+ sizeof(opts_def)); \
+ opts_def.reserved[0] = 0; \
+ opts_def.l4proto = IPPROTO_TCP; \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_einval_reserved = opts_def.error; \
+ opts_def.netns_id = -2; \
+ ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def, \
+ sizeof(opts_def)); \
+ opts_def.netns_id = -1; \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_einval_netns_id = opts_def.error; \
+ ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def, \
+ sizeof(opts_def) - 1); \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_einval_len_opts = opts_def.error; \
+ opts_def.l4proto = IPPROTO_ICMP; \
+ ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def, \
+ sizeof(opts_def)); \
+ opts_def.l4proto = IPPROTO_TCP; \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_eproto_l4proto = opts_def.error; \
+ opts_def.netns_id = 0xf00f; \
+ ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def, \
+ sizeof(opts_def)); \
+ opts_def.netns_id = -1; \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_enonet_netns_id = opts_def.error; \
+ ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def, \
+ sizeof(opts_def)); \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_enoent_lookup = opts_def.error; \
+ ct = func(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4) - 1, \
+ &opts_def, sizeof(opts_def)); \
+ if (ct) \
+ bpf_ct_release(ct); \
+ else \
+ test_eafnosupport = opts_def.error; \
+ })
+
+SEC("xdp")
+int nf_xdp_ct_test(struct xdp_md *ctx)
+{
+ nf_ct_test(bpf_xdp_ct_lookup, ctx);
+ return 0;
+}
+
+SEC("tc")
+int nf_skb_ct_test(struct __sk_buff *ctx)
+{
+ nf_ct_test(bpf_skb_ct_lookup, ctx);
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH bpf-next v5 8/9] selftests/bpf: Add test_verifier support to fixup kfunc call insns
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
` (6 preceding siblings ...)
2021-12-30 2:37 ` [PATCH bpf-next v5 7/9] selftests/bpf: Add test for unstable CT lookup API Kumar Kartikeya Dwivedi
@ 2021-12-30 2:37 ` Kumar Kartikeya Dwivedi
2021-12-30 2:37 ` [PATCH bpf-next v5 9/9] selftests/bpf: Extend kfunc selftests Kumar Kartikeya Dwivedi
2021-12-30 3:04 ` [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:37 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
This allows us to add tests (esp. negative tests) where we only want to
ensure the program doesn't pass through the verifier, and also verify
the error. The next commit will add the tests making use of this.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
tools/testing/selftests/bpf/test_verifier.c | 28 +++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index b0bd2a1f6d52..50a96d01ddb2 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -31,6 +31,7 @@
#include <linux/if_ether.h>
#include <linux/btf.h>
+#include <bpf/btf.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
@@ -66,6 +67,11 @@ static bool unpriv_disabled = false;
static int skips;
static bool verbose = false;
+struct kfunc_btf_id_pair {
+ const char *kfunc;
+ int insn_idx;
+};
+
struct bpf_test {
const char *descr;
struct bpf_insn insns[MAX_INSNS];
@@ -92,6 +98,7 @@ struct bpf_test {
int fixup_map_reuseport_array[MAX_FIXUPS];
int fixup_map_ringbuf[MAX_FIXUPS];
int fixup_map_timer[MAX_FIXUPS];
+ struct kfunc_btf_id_pair fixup_kfunc_btf_id[MAX_FIXUPS];
/* Expected verifier log output for result REJECT or VERBOSE_ACCEPT.
* Can be a tab-separated sequence of expected strings. An empty string
* means no log verification.
@@ -744,6 +751,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
int *fixup_map_reuseport_array = test->fixup_map_reuseport_array;
int *fixup_map_ringbuf = test->fixup_map_ringbuf;
int *fixup_map_timer = test->fixup_map_timer;
+ struct kfunc_btf_id_pair *fixup_kfunc_btf_id = test->fixup_kfunc_btf_id;
if (test->fill_helper) {
test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -936,6 +944,26 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
fixup_map_timer++;
} while (*fixup_map_timer);
}
+
+ /* Patch in kfunc BTF IDs */
+ if (fixup_kfunc_btf_id->kfunc) {
+ struct btf *btf;
+ int btf_id;
+
+ do {
+ btf_id = 0;
+ btf = btf__load_vmlinux_btf();
+ if (btf) {
+ btf_id = btf__find_by_name_kind(btf,
+ fixup_kfunc_btf_id->kfunc,
+ BTF_KIND_FUNC);
+ btf_id = btf_id < 0 ? 0 : btf_id;
+ }
+ btf__free(btf);
+ prog[fixup_kfunc_btf_id->insn_idx].imm = btf_id;
+ fixup_kfunc_btf_id++;
+ } while (fixup_kfunc_btf_id->kfunc);
+ }
}
struct libcap {
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH bpf-next v5 9/9] selftests/bpf: Extend kfunc selftests
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
` (7 preceding siblings ...)
2021-12-30 2:37 ` [PATCH bpf-next v5 8/9] selftests/bpf: Add test_verifier support to fixup kfunc call insns Kumar Kartikeya Dwivedi
@ 2021-12-30 2:37 ` Kumar Kartikeya Dwivedi
2021-12-30 3:04 ` [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 2:37 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
Use the prog_test kfuncs to test the referenced PTR_TO_BTF_ID kfunc
support, and PTR_TO_CTX, PTR_TO_MEM argument passing support. Also
testing the various failure cases for invalid kfunc prototypes.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
net/bpf/test_run.c | 110 ++++++++++++++++++
.../selftests/bpf/prog_tests/kfunc_call.c | 6 +
.../selftests/bpf/progs/kfunc_call_test.c | 52 ++++++++-
tools/testing/selftests/bpf/verifier/calls.c | 75 ++++++++++++
4 files changed, 241 insertions(+), 2 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 2ccd567aa514..665aa7d25929 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -232,6 +232,105 @@ struct sock * noinline bpf_kfunc_call_test3(struct sock *sk)
return sk;
}
+struct prog_test_ref_kfunc {
+ int a;
+ int b;
+ struct prog_test_ref_kfunc *next;
+};
+
+static struct prog_test_ref_kfunc prog_test_struct = {
+ .a = 42,
+ .b = 108,
+ .next = &prog_test_struct,
+};
+
+noinline struct prog_test_ref_kfunc *
+bpf_kfunc_call_test_acquire(unsigned long *scalar_ptr)
+{
+ /* randomly return NULL */
+ if (get_jiffies_64() % 2)
+ return NULL;
+ return &prog_test_struct;
+}
+
+noinline void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p)
+{
+}
+
+struct prog_test_pass1 {
+ int x0;
+ struct {
+ int x1;
+ struct {
+ int x2;
+ struct {
+ int x3;
+ };
+ };
+ };
+};
+
+struct prog_test_pass2 {
+ int len;
+ short arr1[4];
+ struct {
+ char arr2[4];
+ unsigned long arr3[8];
+ } x;
+};
+
+struct prog_test_fail1 {
+ void *p;
+ int x;
+};
+
+struct prog_test_fail2 {
+ int x8;
+ struct prog_test_pass1 x;
+};
+
+struct prog_test_fail3 {
+ int len;
+ char arr1[2];
+ char arr2[0];
+};
+
+noinline void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb)
+{
+}
+
+noinline void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_fail1(struct prog_test_fail1 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_fail2(struct prog_test_fail2 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_fail3(struct prog_test_fail3 *p)
+{
+}
+
+noinline void bpf_kfunc_call_test_mem_len_pass1(void *mem, int mem__sz)
+{
+}
+
+noinline void bpf_kfunc_call_test_mem_len_fail1(void *mem, int len)
+{
+}
+
+noinline void bpf_kfunc_call_test_mem_len_fail2(u64 *mem, int len)
+{
+}
+
__diag_pop();
ALLOW_ERROR_INJECTION(bpf_modify_return_test, ERRNO);
@@ -240,6 +339,17 @@ BTF_KFUNC_SET_START(tc, check, test_sk_kfunc_ids)
BTF_ID(func, bpf_kfunc_call_test1)
BTF_ID(func, bpf_kfunc_call_test2)
BTF_ID(func, bpf_kfunc_call_test3)
+BTF_ID(func, bpf_kfunc_call_test_acquire)
+BTF_ID(func, bpf_kfunc_call_test_release)
+BTF_ID(func, bpf_kfunc_call_test_pass_ctx)
+BTF_ID(func, bpf_kfunc_call_test_pass1)
+BTF_ID(func, bpf_kfunc_call_test_pass2)
+BTF_ID(func, bpf_kfunc_call_test_fail1)
+BTF_ID(func, bpf_kfunc_call_test_fail2)
+BTF_ID(func, bpf_kfunc_call_test_fail3)
+BTF_ID(func, bpf_kfunc_call_test_mem_len_pass1)
+BTF_ID(func, bpf_kfunc_call_test_mem_len_fail1)
+BTF_ID(func, bpf_kfunc_call_test_mem_len_fail2)
BTF_KFUNC_SET_END(tc, check, test_sk_kfunc_ids)
static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index 7d7445ccc141..b39a4f09aefd 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -27,6 +27,12 @@ static void test_main(void)
ASSERT_OK(err, "bpf_prog_test_run(test2)");
ASSERT_EQ(retval, 3, "test2-retval");
+ prog_fd = skel->progs.kfunc_call_test_ref_btf_id.prog_fd;
+ err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
+ NULL, NULL, (__u32 *)&retval, NULL);
+ ASSERT_OK(err, "bpf_prog_test_run(test_ref_btf_id)");
+ ASSERT_EQ(retval, 0, "test_ref_btf_id-retval");
+
kfunc_call_test_lskel__destroy(skel);
}
diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
index 8a8cf59017aa..5aecbb9fdc68 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
@@ -1,13 +1,20 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2021 Facebook */
-#include <linux/bpf.h>
+#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
-#include "bpf_tcp_helpers.h"
extern int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
extern __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
__u32 c, __u64 d) __ksym;
+extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
+extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
+extern void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
+extern void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
+extern void bpf_kfunc_call_test_pass2(struct prog_test_pass2 *p) __ksym;
+extern void bpf_kfunc_call_test_mem_len_pass1(void *mem, int len) __ksym;
+extern void bpf_kfunc_call_test_mem_len_fail2(__u64 *mem, int len) __ksym;
+
SEC("tc")
int kfunc_call_test2(struct __sk_buff *skb)
{
@@ -44,4 +51,45 @@ int kfunc_call_test1(struct __sk_buff *skb)
return ret;
}
+SEC("tc")
+int kfunc_call_test_ref_btf_id(struct __sk_buff *skb)
+{
+ struct prog_test_ref_kfunc *pt;
+ unsigned long s = 0;
+ int ret = 0;
+
+ pt = bpf_kfunc_call_test_acquire(&s);
+ if (pt) {
+ if (pt->a != 42 || pt->b != 108)
+ ret = -1;
+ bpf_kfunc_call_test_release(pt);
+ }
+ return ret;
+}
+
+SEC("tc")
+int kfunc_call_test_pass(struct __sk_buff *skb)
+{
+ struct prog_test_pass1 p1 = {};
+ struct prog_test_pass2 p2 = {};
+ short a = 0;
+ __u64 b = 0;
+ long c = 0;
+ char d = 0;
+ int e = 0;
+
+ bpf_kfunc_call_test_pass_ctx(skb);
+ bpf_kfunc_call_test_pass1(&p1);
+ bpf_kfunc_call_test_pass2(&p2);
+
+ bpf_kfunc_call_test_mem_len_pass1(&a, sizeof(a));
+ bpf_kfunc_call_test_mem_len_pass1(&b, sizeof(b));
+ bpf_kfunc_call_test_mem_len_pass1(&c, sizeof(c));
+ bpf_kfunc_call_test_mem_len_pass1(&d, sizeof(d));
+ bpf_kfunc_call_test_mem_len_pass1(&e, sizeof(e));
+ bpf_kfunc_call_test_mem_len_fail2(&b, -1);
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index d7b74eb28333..829be2b9e08e 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -21,6 +21,81 @@
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
.result = ACCEPT,
},
+{
+ "calls: invalid kfunc call: ptr_to_mem to struct with non-scalar",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = REJECT,
+ .errstr = "arg#0 pointer type STRUCT prog_test_fail1 must point to scalar",
+ .fixup_kfunc_btf_id = {
+ { "bpf_kfunc_call_test_fail1", 2 },
+ },
+},
+{
+ "calls: invalid kfunc call: ptr_to_mem to struct with nesting depth > 4",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = REJECT,
+ .errstr = "max struct nesting depth exceeded\narg#0 pointer type STRUCT prog_test_fail2",
+ .fixup_kfunc_btf_id = {
+ { "bpf_kfunc_call_test_fail2", 2 },
+ },
+},
+{
+ "calls: invalid kfunc call: ptr_to_mem to struct with FAM",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = REJECT,
+ .errstr = "arg#0 pointer type STRUCT prog_test_fail3 must point to scalar",
+ .fixup_kfunc_btf_id = {
+ { "bpf_kfunc_call_test_fail3", 2 },
+ },
+},
+{
+ "calls: invalid kfunc call: reg->type != PTR_TO_CTX",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = REJECT,
+ .errstr = "arg#0 expected pointer to ctx, but got PTR",
+ .fixup_kfunc_btf_id = {
+ { "bpf_kfunc_call_test_pass_ctx", 2 },
+ },
+},
+{
+ "calls: invalid kfunc call: void * not allowed in func proto without mem size arg",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .result = REJECT,
+ .errstr = "arg#0 pointer type UNKNOWN must point to scalar",
+ .fixup_kfunc_btf_id = {
+ { "bpf_kfunc_call_test_mem_len_fail1", 2 },
+ },
+},
{
"calls: basic sanity",
.insns = {
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers
2021-12-30 2:36 [PATCH bpf-next v5 0/9] Introduce unstable CT lookup helpers Kumar Kartikeya Dwivedi
` (8 preceding siblings ...)
2021-12-30 2:37 ` [PATCH bpf-next v5 9/9] selftests/bpf: Extend kfunc selftests Kumar Kartikeya Dwivedi
@ 2021-12-30 3:04 ` Kumar Kartikeya Dwivedi
9 siblings, 0 replies; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-12-30 3:04 UTC (permalink / raw)
To: bpf, netdev, netfilter-devel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
Maxim Mikityanskiy, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen
On Thu, Dec 30, 2021 at 08:06:56AM IST, Kumar Kartikeya Dwivedi wrote:
> This series adds unstable conntrack lookup helpers using BPF kfunc support. The
> patch adding the lookup helper is based off of Maxim's recent patch to aid in
> rebasing their series on top of this, all adjusted to work with module kfuncs [0].
>
> [0]: https://lore.kernel.org/bpf/20211019144655.3483197-8-maximmi@nvidia.com
>
> To enable returning a reference to struct nf_conn, the verifier is extended to
> support reference tracking for PTR_TO_BTF_ID, and kfunc is extended with support
> for working as acquire/release functions, similar to existing BPF helpers. kfunc
> returning pointer (limited to PTR_TO_BTF_ID in the kernel) can also return a
> PTR_TO_BTF_ID_OR_NULL now, typically needed when acquiring a resource can fail.
> kfunc can also receive PTR_TO_CTX and PTR_TO_MEM (with some limitations) as
> arguments now. There is also support for passing a mem, len pair as argument
> to kfunc now. In such cases, passing pointer to unsized type (void) is also
> permitted.
>
> Please see individual commits for details.
>
> Changelog:
> ----------
PR [0] and [1] are needed to make BPF CI green for this set.
[0]: https://github.com/kernel-patches/vmtest/pull/53
[1]: https://github.com/libbpf/libbpf/pull/429
> [...]
--
Kartikeya
^ permalink raw reply [flat|nested] 16+ messages in thread