* [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status
@ 2022-07-21 13:42 Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 01/13] bpf: Introduce 8-byte BTF set Kumar Kartikeya Dwivedi
` (14 more replies)
0 siblings, 15 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
Introduce the following new kfuncs:
- bpf_{xdp,skb}_ct_alloc
- bpf_ct_insert_entry
- bpf_ct_{set,change}_timeout
- bpf_ct_{set,change}_status
The setting of timeout and status on allocated or inserted/looked up CT
is same as the ctnetlink interface, hence code is refactored and shared
with the kfuncs. It is ensured allocated CT cannot be passed to kfuncs
that expected inserted CT, and vice versa. Please see individual patches
for details.
Changelog:
----------
v6 -> v7:
v6: https://lore.kernel.org/bpf/20220719132430.19993-1-memxor@gmail.com
* Use .long to encode flags (Alexei)
* Fix description of KF_RET_NULL in documentation (Toke)
v5 -> v6:
v5: https://lore.kernel.org/bpf/20220623192637.3866852-1-memxor@gmail.com
* Introduce kfunc flags, rework verifier to work with them
* Add documentation for kfuncs
* Add comment explaining TRUSTED_ARGS kfunc flag (Alexei)
* Fix missing offset check for trusted arguments (Alexei)
* Change nf_conntrack test minimum delta value to 8
v4 -> v5:
v4: https://lore.kernel.org/bpf/cover.1653600577.git.lorenzo@kernel.org
* Drop read-only PTR_TO_BTF_ID approach, use struct nf_conn___init (Alexei)
* Drop acquire release pair code that is no longer required (Alexei)
* Disable writes into nf_conn, use dedicated helpers (Florian, Alexei)
* Refactor and share ctnetlink code for setting timeout and status
* Do strict type matching on finding __ref suffix on argument to
prevent passing nf_conn___init as nf_conn (offset = 0, match on walk)
* Remove bpf_ct_opts parameter from bpf_ct_insert_entry
* Update selftests for new additions, add more negative tests
v3 -> v4:
v3: https://lore.kernel.org/bpf/cover.1652870182.git.lorenzo@kernel.org
* split bpf_xdp_ct_add in bpf_xdp_ct_alloc/bpf_skb_ct_alloc and
bpf_ct_insert_entry
* add verifier code to properly populate/configure ct entry
* improve selftests
v2 -> v3:
v2: https://lore.kernel.org/bpf/cover.1652372970.git.lorenzo@kernel.org
* add bpf_xdp_ct_add and bpf_ct_refresh_timeout kfunc helpers
* remove conntrack dependency from selftests
* add support for forcing kfunc args to be referenced and related selftests
v1 -> v2:
v1: https://lore.kernel.org/bpf/1327f8f5696ff2bc60400e8f3b79047914ccc837.1651595019.git.lorenzo@kernel.org
* add bpf_ct_refresh_timeout kfunc selftest
Kumar Kartikeya Dwivedi (10):
bpf: Introduce 8-byte BTF set
tools/resolve_btfids: Add support for 8-byte BTF sets
bpf: Switch to new kfunc flags infrastructure
bpf: Add support for forcing kfunc args to be trusted
bpf: Add documentation for kfuncs
net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup
net: netfilter: Add kfuncs to set and change CT timeout
selftests/bpf: Add verifier tests for trusted kfunc args
selftests/bpf: Add negative tests for new nf_conntrack kfuncs
selftests/bpf: Fix test_verifier failed test in unprivileged mode
Lorenzo Bianconi (3):
net: netfilter: Add kfuncs to allocate and insert CT
net: netfilter: Add kfuncs to set and change CT status
selftests/bpf: Add tests for new nf_conntrack kfuncs
Documentation/bpf/index.rst | 1 +
Documentation/bpf/kfuncs.rst | 170 ++++++++
include/linux/bpf.h | 3 +-
include/linux/btf.h | 65 ++--
include/linux/btf_ids.h | 68 +++-
include/net/netfilter/nf_conntrack_core.h | 19 +
kernel/bpf/btf.c | 123 +++---
kernel/bpf/verifier.c | 14 +-
net/bpf/test_run.c | 75 ++--
net/ipv4/bpf_tcp_ca.c | 18 +-
net/ipv4/tcp_bbr.c | 24 +-
net/ipv4/tcp_cubic.c | 20 +-
net/ipv4/tcp_dctcp.c | 20 +-
net/netfilter/nf_conntrack_bpf.c | 365 +++++++++++++-----
net/netfilter/nf_conntrack_core.c | 62 +++
net/netfilter/nf_conntrack_netlink.c | 54 +--
tools/bpf/resolve_btfids/main.c | 40 +-
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +-
.../testing/selftests/bpf/prog_tests/bpf_nf.c | 64 ++-
.../testing/selftests/bpf/progs/test_bpf_nf.c | 85 +++-
.../selftests/bpf/progs/test_bpf_nf_fail.c | 134 +++++++
.../selftests/bpf/verifier/bpf_loop_inline.c | 1 +
tools/testing/selftests/bpf/verifier/calls.c | 53 +++
23 files changed, 1139 insertions(+), 349 deletions(-)
create mode 100644 Documentation/bpf/kfuncs.rst
create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
--
2.34.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 01/13] bpf: Introduce 8-byte BTF set
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 02/13] tools/resolve_btfids: Add support for 8-byte BTF sets Kumar Kartikeya Dwivedi
` (13 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
Introduce support for defining flags for kfuncs using a new set of
macros, BTF_SET8_START/BTF_SET8_END, which define a set which contains
8 byte elements (each of which consists of a pair of BTF ID and flags),
using a new BTF_ID_FLAGS macro.
This will be used to tag kfuncs registered for a certain program type
as acquire, release, sleepable, ret_null, etc. without having to create
more and more sets which was proving to be an unscalable solution.
Now, when looking up whether a kfunc is allowed for a certain program,
we can also obtain its kfunc flags in the same call and avoid further
lookups.
The resolve_btfids change is split into a separate patch.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/btf_ids.h | 68 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 64 insertions(+), 4 deletions(-)
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index 252a4befeab1..3cb0741e71d7 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -8,6 +8,15 @@ struct btf_id_set {
u32 ids[];
};
+struct btf_id_set8 {
+ u32 cnt;
+ u32 flags;
+ struct {
+ u32 id;
+ u32 flags;
+ } pairs[];
+};
+
#ifdef CONFIG_DEBUG_INFO_BTF
#include <linux/compiler.h> /* for __PASTE */
@@ -25,7 +34,7 @@ struct btf_id_set {
#define BTF_IDS_SECTION ".BTF_ids"
-#define ____BTF_ID(symbol) \
+#define ____BTF_ID(symbol, word) \
asm( \
".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
".local " #symbol " ; \n" \
@@ -33,10 +42,11 @@ asm( \
".size " #symbol ", 4; \n" \
#symbol ": \n" \
".zero 4 \n" \
+word \
".popsection; \n");
-#define __BTF_ID(symbol) \
- ____BTF_ID(symbol)
+#define __BTF_ID(symbol, word) \
+ ____BTF_ID(symbol, word)
#define __ID(prefix) \
__PASTE(prefix, __COUNTER__)
@@ -46,7 +56,14 @@ asm( \
* to 4 zero bytes.
*/
#define BTF_ID(prefix, name) \
- __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
+ __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__), "")
+
+#define ____BTF_ID_FLAGS(prefix, name, flags) \
+ __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__), ".long " #flags "\n")
+#define __BTF_ID_FLAGS(prefix, name, flags, ...) \
+ ____BTF_ID_FLAGS(prefix, name, flags)
+#define BTF_ID_FLAGS(prefix, name, ...) \
+ __BTF_ID_FLAGS(prefix, name, ##__VA_ARGS__, 0)
/*
* The BTF_ID_LIST macro defines pure (unsorted) list
@@ -145,10 +162,51 @@ asm( \
".popsection; \n"); \
extern struct btf_id_set name;
+/*
+ * The BTF_SET8_START/END macros pair defines sorted list of
+ * BTF IDs and their flags plus its members count, with the
+ * following layout:
+ *
+ * BTF_SET8_START(list)
+ * BTF_ID_FLAGS(type1, name1, flags)
+ * BTF_ID_FLAGS(type2, name2, flags)
+ * BTF_SET8_END(list)
+ *
+ * __BTF_ID__set8__list:
+ * .zero 8
+ * list:
+ * __BTF_ID__type1__name1__3:
+ * .zero 4
+ * .word (1 << 0) | (1 << 2)
+ * __BTF_ID__type2__name2__5:
+ * .zero 4
+ * .word (1 << 3) | (1 << 1) | (1 << 2)
+ *
+ */
+#define __BTF_SET8_START(name, scope) \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+"." #scope " __BTF_ID__set8__" #name "; \n" \
+"__BTF_ID__set8__" #name ":; \n" \
+".zero 8 \n" \
+".popsection; \n");
+
+#define BTF_SET8_START(name) \
+__BTF_ID_LIST(name, local) \
+__BTF_SET8_START(name, local)
+
+#define BTF_SET8_END(name) \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+".size __BTF_ID__set8__" #name ", .-" #name " \n" \
+".popsection; \n"); \
+extern struct btf_id_set8 name;
+
#else
#define BTF_ID_LIST(name) static u32 __maybe_unused name[5];
#define BTF_ID(prefix, name)
+#define BTF_ID_FLAGS(prefix, name, flags)
#define BTF_ID_UNUSED
#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];
@@ -156,6 +214,8 @@ extern struct btf_id_set name;
#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_SET8_START(name) static struct btf_id_set8 __maybe_unused name = { 0 };
+#define BTF_SET8_END(name) static struct btf_id_set8 __maybe_unused name = { 0 };
#endif /* CONFIG_DEBUG_INFO_BTF */
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 02/13] tools/resolve_btfids: Add support for 8-byte BTF sets
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 01/13] bpf: Introduce 8-byte BTF set Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 20:51 ` Jiri Olsa
2022-07-21 13:42 ` [PATCH bpf-next v7 03/13] bpf: Switch to new kfunc flags infrastructure Kumar Kartikeya Dwivedi
` (12 subsequent siblings)
14 siblings, 1 reply; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Jiri Olsa, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
A flag is a 4-byte symbol that may follow a BTF ID in a set8. This is
used in the kernel to tag kfuncs in BTF sets with certain flags. Add
support to adjust the sorting code so that it passes size as 8 bytes
for 8-byte BTF sets.
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
tools/bpf/resolve_btfids/main.c | 40 ++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 5d26f3c6f918..80cd7843c677 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -45,6 +45,19 @@
* .zero 4
* __BTF_ID__func__vfs_fallocate__4:
* .zero 4
+ *
+ * set8 - store symbol size into first 4 bytes and sort following
+ * ID list
+ *
+ * __BTF_ID__set8__list:
+ * .zero 8
+ * list:
+ * __BTF_ID__func__vfs_getattr__3:
+ * .zero 4
+ * .word (1 << 0) | (1 << 2)
+ * __BTF_ID__func__vfs_fallocate__5:
+ * .zero 4
+ * .word (1 << 3) | (1 << 1) | (1 << 2)
*/
#define _GNU_SOURCE
@@ -72,6 +85,7 @@
#define BTF_TYPEDEF "typedef"
#define BTF_FUNC "func"
#define BTF_SET "set"
+#define BTF_SET8 "set8"
#define ADDR_CNT 100
@@ -84,6 +98,7 @@ struct btf_id {
};
int addr_cnt;
bool is_set;
+ bool is_set8;
Elf64_Addr addr[ADDR_CNT];
};
@@ -231,14 +246,14 @@ static char *get_id(const char *prefix_end)
return id;
}
-static struct btf_id *add_set(struct object *obj, char *name)
+static struct btf_id *add_set(struct object *obj, char *name, bool is_set8)
{
/*
* __BTF_ID__set__name
* name = ^
* id = ^
*/
- char *id = name + sizeof(BTF_SET "__") - 1;
+ char *id = name + (is_set8 ? sizeof(BTF_SET8 "__") : sizeof(BTF_SET "__")) - 1;
int len = strlen(name);
if (id >= name + len) {
@@ -444,9 +459,21 @@ static int symbols_collect(struct object *obj)
} else if (!strncmp(prefix, BTF_FUNC, sizeof(BTF_FUNC) - 1)) {
obj->nr_funcs++;
id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1);
+ /* set8 */
+ } else if (!strncmp(prefix, BTF_SET8, sizeof(BTF_SET8) - 1)) {
+ id = add_set(obj, prefix, true);
+ /*
+ * SET8 objects store list's count, which is encoded
+ * in symbol's size, together with 'cnt' field hence
+ * that - 1.
+ */
+ if (id) {
+ id->cnt = sym.st_size / sizeof(uint64_t) - 1;
+ id->is_set8 = true;
+ }
/* set */
} else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) {
- id = add_set(obj, prefix);
+ id = add_set(obj, prefix, false);
/*
* SET objects store list's count, which is encoded
* in symbol's size, together with 'cnt' field hence
@@ -571,7 +598,8 @@ static int id_patch(struct object *obj, struct btf_id *id)
int *ptr = data->d_buf;
int i;
- if (!id->id && !id->is_set)
+ /* For set, set8, id->id may be 0 */
+ if (!id->id && !id->is_set && !id->is_set8)
pr_err("WARN: resolve_btfids: unresolved symbol %s\n", id->name);
for (i = 0; i < id->addr_cnt; i++) {
@@ -643,13 +671,13 @@ static int sets_patch(struct object *obj)
}
idx = idx / sizeof(int);
- base = &ptr[idx] + 1;
+ base = &ptr[idx] + (id->is_set8 ? 2 : 1);
cnt = ptr[idx];
pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
(idx + 1) * sizeof(int), cnt, id->name);
- qsort(base, cnt, sizeof(int), cmp_id);
+ qsort(base, cnt, id->is_set8 ? sizeof(uint64_t) : sizeof(int), cmp_id);
next = rb_next(next);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 03/13] bpf: Switch to new kfunc flags infrastructure
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 01/13] bpf: Introduce 8-byte BTF set Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 02/13] tools/resolve_btfids: Add support for 8-byte BTF sets Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted Kumar Kartikeya Dwivedi
` (11 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
Instead of populating multiple sets to indicate some attribute and then
researching the same BTF ID in them, prepare a single unified BTF set
which indicates whether a kfunc is allowed to be called, and also its
attributes if any at the same time. Now, only one call is needed to
perform the lookup for both kfunc availability and its attributes.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/bpf.h | 3 +-
include/linux/btf.h | 33 ++----
kernel/bpf/btf.c | 106 ++++++++----------
kernel/bpf/verifier.c | 14 +--
net/bpf/test_run.c | 70 ++++--------
net/ipv4/bpf_tcp_ca.c | 18 +--
net/ipv4/tcp_bbr.c | 24 ++--
net/ipv4/tcp_cubic.c | 20 ++--
net/ipv4/tcp_dctcp.c | 20 ++--
net/netfilter/nf_conntrack_bpf.c | 49 ++------
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +-
11 files changed, 145 insertions(+), 222 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a5bf00649995..974b575eb9e3 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1920,7 +1920,8 @@ int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
struct bpf_reg_state *regs);
int btf_check_kfunc_arg_match(struct bpf_verifier_env *env,
const struct btf *btf, u32 func_id,
- struct bpf_reg_state *regs);
+ struct bpf_reg_state *regs,
+ u32 kfunc_flags);
int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog,
struct bpf_reg_state *reg);
int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *prog,
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 1bfed7fa0428..6dfc6eaf7f8c 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -12,14 +12,11 @@
#define BTF_TYPE_EMIT(type) ((void)(type *)0)
#define BTF_TYPE_EMIT_ENUM(enum_val) ((void)enum_val)
-enum btf_kfunc_type {
- BTF_KFUNC_TYPE_CHECK,
- BTF_KFUNC_TYPE_ACQUIRE,
- BTF_KFUNC_TYPE_RELEASE,
- BTF_KFUNC_TYPE_RET_NULL,
- BTF_KFUNC_TYPE_KPTR_ACQUIRE,
- BTF_KFUNC_TYPE_MAX,
-};
+/* These need to be macros, as the expressions are used in assembler input */
+#define KF_ACQUIRE (1 << 0) /* kfunc is an acquire function */
+#define KF_RELEASE (1 << 1) /* kfunc is a release function */
+#define KF_RET_NULL (1 << 2) /* kfunc returns a pointer that may be NULL */
+#define KF_KPTR_GET (1 << 3) /* kfunc returns reference to a kptr */
struct btf;
struct btf_member;
@@ -30,16 +27,7 @@ struct btf_id_set;
struct btf_kfunc_id_set {
struct module *owner;
- union {
- struct {
- struct btf_id_set *check_set;
- struct btf_id_set *acquire_set;
- struct btf_id_set *release_set;
- struct btf_id_set *ret_null_set;
- struct btf_id_set *kptr_acquire_set;
- };
- struct btf_id_set *sets[BTF_KFUNC_TYPE_MAX];
- };
+ struct btf_id_set8 *set;
};
struct btf_id_dtor_kfunc {
@@ -378,9 +366,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,
+u32 *btf_kfunc_id_set_contains(const struct btf *btf,
enum bpf_prog_type prog_type,
- enum btf_kfunc_type type, u32 kfunc_btf_id);
+ u32 kfunc_btf_id);
int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
const struct btf_kfunc_id_set *s);
s32 btf_find_dtor_kfunc(struct btf *btf, u32 btf_id);
@@ -397,12 +385,11 @@ 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,
+static inline u32 *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;
+ return NULL;
}
static inline int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
const struct btf_kfunc_id_set *s)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 5869f03bcb6e..4d9c2d88720f 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -213,7 +213,7 @@ enum {
};
struct btf_kfunc_set_tab {
- struct btf_id_set *sets[BTF_KFUNC_HOOK_MAX][BTF_KFUNC_TYPE_MAX];
+ struct btf_id_set8 *sets[BTF_KFUNC_HOOK_MAX];
};
struct btf_id_dtor_kfunc_tab {
@@ -1616,7 +1616,7 @@ static void btf_free_id(struct btf *btf)
static void btf_free_kfunc_set_tab(struct btf *btf)
{
struct btf_kfunc_set_tab *tab = btf->kfunc_set_tab;
- int hook, type;
+ int hook;
if (!tab)
return;
@@ -1625,10 +1625,8 @@ static void btf_free_kfunc_set_tab(struct btf *btf)
*/
if (btf_is_module(btf))
goto free_tab;
- for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++) {
- for (type = 0; type < ARRAY_SIZE(tab->sets[0]); type++)
- kfree(tab->sets[hook][type]);
- }
+ for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++)
+ kfree(tab->sets[hook]);
free_tab:
kfree(tab);
btf->kfunc_set_tab = NULL;
@@ -6172,7 +6170,8 @@ static bool is_kfunc_arg_mem_size(const struct btf *btf,
static int btf_check_func_arg_match(struct bpf_verifier_env *env,
const struct btf *btf, u32 func_id,
struct bpf_reg_state *regs,
- bool ptr_to_mem_ok)
+ bool ptr_to_mem_ok,
+ u32 kfunc_flags)
{
enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
struct bpf_verifier_log *log = &env->log;
@@ -6210,10 +6209,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
if (is_kfunc) {
/* Only kfunc can be release func */
- rel = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
- BTF_KFUNC_TYPE_RELEASE, func_id);
- kptr_get = btf_kfunc_id_set_contains(btf, resolve_prog_type(env->prog),
- BTF_KFUNC_TYPE_KPTR_ACQUIRE, func_id);
+ rel = kfunc_flags & KF_RELEASE;
+ kptr_get = kfunc_flags & KF_KPTR_GET;
}
/* check that BTF function arguments match actual types that the
@@ -6442,7 +6439,7 @@ int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
return -EINVAL;
is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
- err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global);
+ err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global, 0);
/* Compiler optimizations can remove arguments from static functions
* or mismatched type can be passed into a global function.
@@ -6455,9 +6452,10 @@ int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
int btf_check_kfunc_arg_match(struct bpf_verifier_env *env,
const struct btf *btf, u32 func_id,
- struct bpf_reg_state *regs)
+ struct bpf_reg_state *regs,
+ u32 kfunc_flags)
{
- return btf_check_func_arg_match(env, btf, func_id, regs, true);
+ return btf_check_func_arg_match(env, btf, func_id, regs, true, kfunc_flags);
}
/* Convert BTF of a function into bpf_reg_state if possible
@@ -6854,6 +6852,11 @@ bool btf_id_set_contains(const struct btf_id_set *set, u32 id)
return bsearch(&id, set->ids, set->cnt, sizeof(u32), btf_id_cmp_func) != NULL;
}
+static void *btf_id_set8_contains(const struct btf_id_set8 *set, u32 id)
+{
+ return bsearch(&id, set->pairs, set->cnt, sizeof(set->pairs[0]), btf_id_cmp_func);
+}
+
enum {
BTF_MODULE_F_LIVE = (1 << 0),
};
@@ -7102,16 +7105,16 @@ BTF_TRACING_TYPE_xxx
/* Kernel Function (kfunc) BTF ID set registration API */
-static int __btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
- enum btf_kfunc_type type,
- struct btf_id_set *add_set, bool vmlinux_set)
+static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
+ struct btf_id_set8 *add_set)
{
+ bool vmlinux_set = !btf_is_module(btf);
struct btf_kfunc_set_tab *tab;
- struct btf_id_set *set;
+ struct btf_id_set8 *set;
u32 set_cnt;
int ret;
- if (hook >= BTF_KFUNC_HOOK_MAX || type >= BTF_KFUNC_TYPE_MAX) {
+ if (hook >= BTF_KFUNC_HOOK_MAX) {
ret = -EINVAL;
goto end;
}
@@ -7127,7 +7130,7 @@ static int __btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
btf->kfunc_set_tab = tab;
}
- set = tab->sets[hook][type];
+ set = tab->sets[hook];
/* Warn when register_btf_kfunc_id_set is called twice for the same hook
* for module sets.
*/
@@ -7141,7 +7144,7 @@ static int __btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
* pointer and return.
*/
if (!vmlinux_set) {
- tab->sets[hook][type] = add_set;
+ tab->sets[hook] = add_set;
return 0;
}
@@ -7150,7 +7153,7 @@ static int __btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
* and concatenate all individual sets being registered. While each set
* is individually sorted, they may become unsorted when concatenated,
* hence re-sorting the final set again is required to make binary
- * searching the set using btf_id_set_contains function work.
+ * searching the set using btf_id_set8_contains function work.
*/
set_cnt = set ? set->cnt : 0;
@@ -7165,8 +7168,8 @@ static int __btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
}
/* Grow set */
- set = krealloc(tab->sets[hook][type],
- offsetof(struct btf_id_set, ids[set_cnt + add_set->cnt]),
+ set = krealloc(tab->sets[hook],
+ offsetof(struct btf_id_set8, pairs[set_cnt + add_set->cnt]),
GFP_KERNEL | __GFP_NOWARN);
if (!set) {
ret = -ENOMEM;
@@ -7174,15 +7177,15 @@ static int __btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
}
/* For newly allocated set, initialize set->cnt to 0 */
- if (!tab->sets[hook][type])
+ if (!tab->sets[hook])
set->cnt = 0;
- tab->sets[hook][type] = set;
+ tab->sets[hook] = set;
/* Concatenate the two sets */
- memcpy(set->ids + set->cnt, add_set->ids, add_set->cnt * sizeof(set->ids[0]));
+ memcpy(set->pairs + set->cnt, add_set->pairs, add_set->cnt * sizeof(set->pairs[0]));
set->cnt += add_set->cnt;
- sort(set->ids, set->cnt, sizeof(set->ids[0]), btf_id_cmp_func, NULL);
+ sort(set->pairs, set->cnt, sizeof(set->pairs[0]), btf_id_cmp_func, NULL);
return 0;
end:
@@ -7190,38 +7193,25 @@ static int __btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
return ret;
}
-static int btf_populate_kfunc_set(struct btf *btf, enum btf_kfunc_hook hook,
- const struct btf_kfunc_id_set *kset)
-{
- bool vmlinux_set = !btf_is_module(btf);
- int type, ret = 0;
-
- for (type = 0; type < ARRAY_SIZE(kset->sets); type++) {
- if (!kset->sets[type])
- continue;
-
- ret = __btf_populate_kfunc_set(btf, hook, type, kset->sets[type], vmlinux_set);
- if (ret)
- break;
- }
- return ret;
-}
-
-static bool __btf_kfunc_id_set_contains(const struct btf *btf,
+static u32 *__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;
+ struct btf_id_set8 *set;
+ u32 *id;
- if (hook >= BTF_KFUNC_HOOK_MAX || type >= BTF_KFUNC_TYPE_MAX)
- return false;
+ if (hook >= BTF_KFUNC_HOOK_MAX)
+ return NULL;
if (!btf->kfunc_set_tab)
- return false;
- set = btf->kfunc_set_tab->sets[hook][type];
+ return NULL;
+ set = btf->kfunc_set_tab->sets[hook];
if (!set)
- return false;
- return btf_id_set_contains(set, kfunc_btf_id);
+ return NULL;
+ id = btf_id_set8_contains(set, kfunc_btf_id);
+ if (!id)
+ return NULL;
+ /* The flags for BTF ID are located next to it */
+ return id + 1;
}
static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
@@ -7249,14 +7239,14 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
* keeping the reference for the duration of the call provides the necessary
* protection for looking up a well-formed btf->kfunc_set_tab.
*/
-bool btf_kfunc_id_set_contains(const struct btf *btf,
+u32 *btf_kfunc_id_set_contains(const struct btf *btf,
enum bpf_prog_type prog_type,
- enum btf_kfunc_type type, u32 kfunc_btf_id)
+ u32 kfunc_btf_id)
{
enum btf_kfunc_hook hook;
hook = bpf_prog_type_to_kfunc_hook(prog_type);
- return __btf_kfunc_id_set_contains(btf, hook, type, kfunc_btf_id);
+ return __btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
}
/* This function must be invoked only from initcalls/module init functions */
@@ -7283,7 +7273,7 @@ int register_btf_kfunc_id_set(enum bpf_prog_type prog_type,
return PTR_ERR(btf);
hook = bpf_prog_type_to_kfunc_hook(prog_type);
- ret = btf_populate_kfunc_set(btf, hook, kset);
+ ret = btf_populate_kfunc_set(btf, hook, kset->set);
btf_put(btf);
return ret;
}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c59c3df0fea6..6a12e3613d22 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7560,6 +7560,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
int err, insn_idx = *insn_idx_p;
const struct btf_param *args;
struct btf *desc_btf;
+ u32 *kfunc_flags;
bool acq;
/* skip for now, but return error when we find this in fixup_kfunc_call */
@@ -7575,18 +7576,16 @@ 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 (!btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog),
- BTF_KFUNC_TYPE_CHECK, func_id)) {
+ kfunc_flags = btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog), func_id);
+ if (!kfunc_flags) {
verbose(env, "calling kernel function %s is not allowed\n",
func_name);
return -EACCES;
}
-
- acq = btf_kfunc_id_set_contains(desc_btf, resolve_prog_type(env->prog),
- BTF_KFUNC_TYPE_ACQUIRE, func_id);
+ acq = *kfunc_flags & KF_ACQUIRE;
/* Check the arguments */
- err = btf_check_kfunc_arg_match(env, desc_btf, func_id, regs);
+ err = btf_check_kfunc_arg_match(env, desc_btf, func_id, regs, *kfunc_flags);
if (err < 0)
return err;
/* In case of release function, we get register number of refcounted
@@ -7630,8 +7629,7 @@ 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)) {
+ if (*kfunc_flags & KF_RET_NULL) {
regs[BPF_REG_0].type |= PTR_MAYBE_NULL;
/* For mark_ptr_or_null_reg, see 93c230e3f5bd6 */
regs[BPF_REG_0].id = ++env->id_gen;
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index dc9dc0bedca0..ca5b7234a350 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -695,48 +695,26 @@ __diag_pop();
ALLOW_ERROR_INJECTION(bpf_modify_return_test, ERRNO);
-BTF_SET_START(test_sk_check_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_memb_acquire)
-BTF_ID(func, bpf_kfunc_call_test_release)
-BTF_ID(func, bpf_kfunc_call_memb_release)
-BTF_ID(func, bpf_kfunc_call_memb1_release)
-BTF_ID(func, bpf_kfunc_call_test_kptr_get)
-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_SET_END(test_sk_check_kfunc_ids)
-
-BTF_SET_START(test_sk_acquire_kfunc_ids)
-BTF_ID(func, bpf_kfunc_call_test_acquire)
-BTF_ID(func, bpf_kfunc_call_memb_acquire)
-BTF_ID(func, bpf_kfunc_call_test_kptr_get)
-BTF_SET_END(test_sk_acquire_kfunc_ids)
-
-BTF_SET_START(test_sk_release_kfunc_ids)
-BTF_ID(func, bpf_kfunc_call_test_release)
-BTF_ID(func, bpf_kfunc_call_memb_release)
-BTF_ID(func, bpf_kfunc_call_memb1_release)
-BTF_SET_END(test_sk_release_kfunc_ids)
-
-BTF_SET_START(test_sk_ret_null_kfunc_ids)
-BTF_ID(func, bpf_kfunc_call_test_acquire)
-BTF_ID(func, bpf_kfunc_call_memb_acquire)
-BTF_ID(func, bpf_kfunc_call_test_kptr_get)
-BTF_SET_END(test_sk_ret_null_kfunc_ids)
-
-BTF_SET_START(test_sk_kptr_acquire_kfunc_ids)
-BTF_ID(func, bpf_kfunc_call_test_kptr_get)
-BTF_SET_END(test_sk_kptr_acquire_kfunc_ids)
+BTF_SET8_START(test_sk_check_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_acquire, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_call_memb_acquire, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_call_memb_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_call_memb1_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_kptr_get, KF_ACQUIRE | KF_RET_NULL | KF_KPTR_GET)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass_ctx)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail3)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
+BTF_SET8_END(test_sk_check_kfunc_ids)
static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
u32 size, u32 headroom, u32 tailroom)
@@ -1620,12 +1598,8 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
}
static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &test_sk_check_kfunc_ids,
- .acquire_set = &test_sk_acquire_kfunc_ids,
- .release_set = &test_sk_release_kfunc_ids,
- .ret_null_set = &test_sk_ret_null_kfunc_ids,
- .kptr_acquire_set = &test_sk_kptr_acquire_kfunc_ids
+ .owner = THIS_MODULE,
+ .set = &test_sk_check_kfunc_ids,
};
BTF_ID_LIST(bpf_prog_test_dtor_kfunc_ids)
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
index 7a181631b995..85a9e500c42d 100644
--- a/net/ipv4/bpf_tcp_ca.c
+++ b/net/ipv4/bpf_tcp_ca.c
@@ -197,17 +197,17 @@ bpf_tcp_ca_get_func_proto(enum bpf_func_id func_id,
}
}
-BTF_SET_START(bpf_tcp_ca_check_kfunc_ids)
-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_check_kfunc_ids)
+BTF_SET8_START(bpf_tcp_ca_check_kfunc_ids)
+BTF_ID_FLAGS(func, tcp_reno_ssthresh)
+BTF_ID_FLAGS(func, tcp_reno_cong_avoid)
+BTF_ID_FLAGS(func, tcp_reno_undo_cwnd)
+BTF_ID_FLAGS(func, tcp_slow_start)
+BTF_ID_FLAGS(func, tcp_cong_avoid_ai)
+BTF_SET8_END(bpf_tcp_ca_check_kfunc_ids)
static const struct btf_kfunc_id_set bpf_tcp_ca_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &bpf_tcp_ca_check_kfunc_ids,
+ .owner = THIS_MODULE,
+ .set = &bpf_tcp_ca_check_kfunc_ids,
};
static const struct bpf_verifier_ops bpf_tcp_ca_verifier_ops = {
diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
index 075e744bfb48..54eec33c6e1c 100644
--- a/net/ipv4/tcp_bbr.c
+++ b/net/ipv4/tcp_bbr.c
@@ -1154,24 +1154,24 @@ static struct tcp_congestion_ops tcp_bbr_cong_ops __read_mostly = {
.set_state = bbr_set_state,
};
-BTF_SET_START(tcp_bbr_check_kfunc_ids)
+BTF_SET8_START(tcp_bbr_check_kfunc_ids)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
-BTF_ID(func, bbr_init)
-BTF_ID(func, bbr_main)
-BTF_ID(func, bbr_sndbuf_expand)
-BTF_ID(func, bbr_undo_cwnd)
-BTF_ID(func, bbr_cwnd_event)
-BTF_ID(func, bbr_ssthresh)
-BTF_ID(func, bbr_min_tso_segs)
-BTF_ID(func, bbr_set_state)
+BTF_ID_FLAGS(func, bbr_init)
+BTF_ID_FLAGS(func, bbr_main)
+BTF_ID_FLAGS(func, bbr_sndbuf_expand)
+BTF_ID_FLAGS(func, bbr_undo_cwnd)
+BTF_ID_FLAGS(func, bbr_cwnd_event)
+BTF_ID_FLAGS(func, bbr_ssthresh)
+BTF_ID_FLAGS(func, bbr_min_tso_segs)
+BTF_ID_FLAGS(func, bbr_set_state)
#endif
#endif
-BTF_SET_END(tcp_bbr_check_kfunc_ids)
+BTF_SET8_END(tcp_bbr_check_kfunc_ids)
static const struct btf_kfunc_id_set tcp_bbr_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &tcp_bbr_check_kfunc_ids,
+ .owner = THIS_MODULE,
+ .set = &tcp_bbr_check_kfunc_ids,
};
static int __init bbr_register(void)
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 68178e7280ce..768c10c1f649 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -485,22 +485,22 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
.name = "cubic",
};
-BTF_SET_START(tcp_cubic_check_kfunc_ids)
+BTF_SET8_START(tcp_cubic_check_kfunc_ids)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
-BTF_ID(func, cubictcp_init)
-BTF_ID(func, cubictcp_recalc_ssthresh)
-BTF_ID(func, cubictcp_cong_avoid)
-BTF_ID(func, cubictcp_state)
-BTF_ID(func, cubictcp_cwnd_event)
-BTF_ID(func, cubictcp_acked)
+BTF_ID_FLAGS(func, cubictcp_init)
+BTF_ID_FLAGS(func, cubictcp_recalc_ssthresh)
+BTF_ID_FLAGS(func, cubictcp_cong_avoid)
+BTF_ID_FLAGS(func, cubictcp_state)
+BTF_ID_FLAGS(func, cubictcp_cwnd_event)
+BTF_ID_FLAGS(func, cubictcp_acked)
#endif
#endif
-BTF_SET_END(tcp_cubic_check_kfunc_ids)
+BTF_SET8_END(tcp_cubic_check_kfunc_ids)
static const struct btf_kfunc_id_set tcp_cubic_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &tcp_cubic_check_kfunc_ids,
+ .owner = THIS_MODULE,
+ .set = &tcp_cubic_check_kfunc_ids,
};
static int __init cubictcp_register(void)
diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
index ab034a4e9324..2a6c0dd665a4 100644
--- a/net/ipv4/tcp_dctcp.c
+++ b/net/ipv4/tcp_dctcp.c
@@ -239,22 +239,22 @@ static struct tcp_congestion_ops dctcp_reno __read_mostly = {
.name = "dctcp-reno",
};
-BTF_SET_START(tcp_dctcp_check_kfunc_ids)
+BTF_SET8_START(tcp_dctcp_check_kfunc_ids)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
-BTF_ID(func, dctcp_init)
-BTF_ID(func, dctcp_update_alpha)
-BTF_ID(func, dctcp_cwnd_event)
-BTF_ID(func, dctcp_ssthresh)
-BTF_ID(func, dctcp_cwnd_undo)
-BTF_ID(func, dctcp_state)
+BTF_ID_FLAGS(func, dctcp_init)
+BTF_ID_FLAGS(func, dctcp_update_alpha)
+BTF_ID_FLAGS(func, dctcp_cwnd_event)
+BTF_ID_FLAGS(func, dctcp_ssthresh)
+BTF_ID_FLAGS(func, dctcp_cwnd_undo)
+BTF_ID_FLAGS(func, dctcp_state)
#endif
#endif
-BTF_SET_END(tcp_dctcp_check_kfunc_ids)
+BTF_SET8_END(tcp_dctcp_check_kfunc_ids)
static const struct btf_kfunc_id_set tcp_dctcp_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &tcp_dctcp_check_kfunc_ids,
+ .owner = THIS_MODULE,
+ .set = &tcp_dctcp_check_kfunc_ids,
};
static int __init dctcp_register(void)
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index bc4d5cd63a94..cf2096f65d0e 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -219,48 +219,21 @@ void bpf_ct_release(struct nf_conn *nfct)
__diag_pop()
-BTF_SET_START(nf_ct_xdp_check_kfunc_ids)
-BTF_ID(func, bpf_xdp_ct_lookup)
-BTF_ID(func, bpf_ct_release)
-BTF_SET_END(nf_ct_xdp_check_kfunc_ids)
-
-BTF_SET_START(nf_ct_tc_check_kfunc_ids)
-BTF_ID(func, bpf_skb_ct_lookup)
-BTF_ID(func, bpf_ct_release)
-BTF_SET_END(nf_ct_tc_check_kfunc_ids)
-
-BTF_SET_START(nf_ct_acquire_kfunc_ids)
-BTF_ID(func, bpf_xdp_ct_lookup)
-BTF_ID(func, bpf_skb_ct_lookup)
-BTF_SET_END(nf_ct_acquire_kfunc_ids)
-
-BTF_SET_START(nf_ct_release_kfunc_ids)
-BTF_ID(func, bpf_ct_release)
-BTF_SET_END(nf_ct_release_kfunc_ids)
-
-/* Both sets are identical */
-#define nf_ct_ret_null_kfunc_ids nf_ct_acquire_kfunc_ids
-
-static const struct btf_kfunc_id_set nf_conntrack_xdp_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &nf_ct_xdp_check_kfunc_ids,
- .acquire_set = &nf_ct_acquire_kfunc_ids,
- .release_set = &nf_ct_release_kfunc_ids,
- .ret_null_set = &nf_ct_ret_null_kfunc_ids,
-};
-
-static const struct btf_kfunc_id_set nf_conntrack_tc_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &nf_ct_tc_check_kfunc_ids,
- .acquire_set = &nf_ct_acquire_kfunc_ids,
- .release_set = &nf_ct_release_kfunc_ids,
- .ret_null_set = &nf_ct_ret_null_kfunc_ids,
+BTF_SET8_START(nf_ct_kfunc_set)
+BTF_ID_FLAGS(func, bpf_xdp_ct_lookup, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_skb_ct_lookup, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_ct_release, KF_RELEASE)
+BTF_SET8_END(nf_ct_kfunc_set)
+
+static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &nf_ct_kfunc_set,
};
int register_nf_conntrack_bpf(void)
{
int ret;
- ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &nf_conntrack_xdp_kfunc_set);
- return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &nf_conntrack_tc_kfunc_set);
+ ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &nf_conntrack_kfunc_set);
+ return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &nf_conntrack_kfunc_set);
}
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index e585e1cefc77..792cb15bac40 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -148,13 +148,13 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
.write = bpf_testmod_test_write,
};
-BTF_SET_START(bpf_testmod_check_kfunc_ids)
-BTF_ID(func, bpf_testmod_test_mod_kfunc)
-BTF_SET_END(bpf_testmod_check_kfunc_ids)
+BTF_SET8_START(bpf_testmod_check_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_testmod_test_mod_kfunc)
+BTF_SET8_END(bpf_testmod_check_kfunc_ids)
static const struct btf_kfunc_id_set bpf_testmod_kfunc_set = {
- .owner = THIS_MODULE,
- .check_set = &bpf_testmod_check_kfunc_ids,
+ .owner = THIS_MODULE,
+ .set = &bpf_testmod_check_kfunc_ids,
};
extern int bpf_fentry_test1(int a);
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (2 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 03/13] bpf: Switch to new kfunc flags infrastructure Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-22 4:10 ` Alexei Starovoitov
2022-07-25 9:52 ` Roberto Sassu
2022-07-21 13:42 ` [PATCH bpf-next v7 05/13] bpf: Add documentation for kfuncs Kumar Kartikeya Dwivedi
` (10 subsequent siblings)
14 siblings, 2 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
Teach the verifier to detect a new KF_TRUSTED_ARGS kfunc flag, which
means each pointer argument must be trusted, which we define as a
pointer that is referenced (has non-zero ref_obj_id) and also needs to
have its offset unchanged, similar to how release functions expect their
argument. This allows a kfunc to receive pointer arguments unchanged
from the result of the acquire kfunc.
This is required to ensure that kfunc that operate on some object only
work on acquired pointers and not normal PTR_TO_BTF_ID with same type
which can be obtained by pointer walking. The restrictions applied to
release arguments also apply to trusted arguments. This implies that
strict type matching (not deducing type by recursively following members
at offset) and OBJ_RELEASE offset checks (ensuring they are zero) are
used for trusted pointer arguments.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/btf.h | 32 ++++++++++++++++++++++++++++++++
kernel/bpf/btf.c | 17 ++++++++++++++---
net/bpf/test_run.c | 5 +++++
3 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 6dfc6eaf7f8c..cb63aa71e82f 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -17,6 +17,38 @@
#define KF_RELEASE (1 << 1) /* kfunc is a release function */
#define KF_RET_NULL (1 << 2) /* kfunc returns a pointer that may be NULL */
#define KF_KPTR_GET (1 << 3) /* kfunc returns reference to a kptr */
+/* Trusted arguments are those which are meant to be referenced arguments with
+ * unchanged offset. It is used to enforce that pointers obtained from acquire
+ * kfuncs remain unmodified when being passed to helpers taking trusted args.
+ *
+ * Consider
+ * struct foo {
+ * int data;
+ * struct foo *next;
+ * };
+ *
+ * struct bar {
+ * int data;
+ * struct foo f;
+ * };
+ *
+ * struct foo *f = alloc_foo(); // Acquire kfunc
+ * struct bar *b = alloc_bar(); // Acquire kfunc
+ *
+ * If a kfunc set_foo_data() wants to operate only on the allocated object, it
+ * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage like:
+ *
+ * set_foo_data(f, 42); // Allowed
+ * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
+ * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
+ * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
+ *
+ * In the final case, usually for the purposes of type matching, it is deduced
+ * by looking at the type of the member at the offset, but due to the
+ * requirement of trusted argument, this deduction will be strict and not done
+ * for this case.
+ */
+#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer arguments */
struct btf;
struct btf_member;
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 4d9c2d88720f..7ac971ea98d1 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6174,10 +6174,10 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
u32 kfunc_flags)
{
enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
+ bool rel = false, kptr_get = false, trusted_arg = false;
struct bpf_verifier_log *log = &env->log;
u32 i, nargs, ref_id, ref_obj_id = 0;
bool is_kfunc = btf_is_kernel(btf);
- bool rel = false, kptr_get = false;
const char *func_name, *ref_tname;
const struct btf_type *t, *ref_t;
const struct btf_param *args;
@@ -6211,6 +6211,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
/* Only kfunc can be release func */
rel = kfunc_flags & KF_RELEASE;
kptr_get = kfunc_flags & KF_KPTR_GET;
+ trusted_arg = kfunc_flags & KF_TRUSTED_ARGS;
}
/* check that BTF function arguments match actual types that the
@@ -6235,10 +6236,19 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
return -EINVAL;
}
+ /* Check if argument must be a referenced pointer, args + i has
+ * been verified to be a pointer (after skipping modifiers).
+ */
+ if (is_kfunc && trusted_arg && !reg->ref_obj_id) {
+ bpf_log(log, "R%d must be referenced\n", regno);
+ return -EINVAL;
+ }
+
ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id);
ref_tname = btf_name_by_offset(btf, ref_t->name_off);
- if (rel && reg->ref_obj_id)
+ /* Trusted args have the same offset checks as release arguments */
+ if (trusted_arg || (rel && reg->ref_obj_id))
arg_type |= OBJ_RELEASE;
ret = check_func_arg_reg_off(env, reg, regno, arg_type);
if (ret < 0)
@@ -6336,7 +6346,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
reg_ref_tname = btf_name_by_offset(reg_btf,
reg_ref_t->name_off);
if (!btf_struct_ids_match(log, reg_btf, reg_ref_id,
- reg->off, btf, ref_id, rel && reg->ref_obj_id)) {
+ reg->off, btf, ref_id,
+ trusted_arg || (rel && reg->ref_obj_id))) {
bpf_log(log, "kernel function %s args#%d expected pointer to %s %s but R%d has a pointer to %s %s\n",
func_name, i,
btf_type_str(ref_t), ref_tname,
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index ca5b7234a350..cbc9cd5058cb 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -691,6 +691,10 @@ noinline void bpf_kfunc_call_test_mem_len_fail2(u64 *mem, int len)
{
}
+noinline void bpf_kfunc_call_test_ref(struct prog_test_ref_kfunc *p)
+{
+}
+
__diag_pop();
ALLOW_ERROR_INJECTION(bpf_modify_return_test, ERRNO);
@@ -714,6 +718,7 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_fail3)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test_ref, KF_TRUSTED_ARGS)
BTF_SET8_END(test_sk_check_kfunc_ids)
static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 05/13] bpf: Add documentation for kfuncs
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (3 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 06/13] net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup Kumar Kartikeya Dwivedi
` (9 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: KP Singh, Jonathan Corbet, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen,
Lorenzo Bianconi, netdev, netfilter-devel
As the usage of kfuncs grows, we are starting to form consensus on the
kinds of attributes and annotations that kfuncs can have. To better help
developers make sense of the various options available at their disposal
to present an unstable API to the BPF users, document the various kfunc
flags and annotations, their expected usage, and explain the process of
defining and registering a kfunc set.
Cc: KP Singh <kpsingh@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
Documentation/bpf/index.rst | 1 +
Documentation/bpf/kfuncs.rst | 170 +++++++++++++++++++++++++++++++++++
2 files changed, 171 insertions(+)
create mode 100644 Documentation/bpf/kfuncs.rst
diff --git a/Documentation/bpf/index.rst b/Documentation/bpf/index.rst
index 96056a7447c7..1bc2c5c58bdb 100644
--- a/Documentation/bpf/index.rst
+++ b/Documentation/bpf/index.rst
@@ -19,6 +19,7 @@ that goes into great technical depth about the BPF Architecture.
faq
syscall_api
helpers
+ kfuncs
programs
maps
bpf_prog_run
diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
new file mode 100644
index 000000000000..c0b7dae6dbf5
--- /dev/null
+++ b/Documentation/bpf/kfuncs.rst
@@ -0,0 +1,170 @@
+=============================
+BPF Kernel Functions (kfuncs)
+=============================
+
+1. Introduction
+===============
+
+BPF Kernel Functions or more commonly known as kfuncs are functions in the Linux
+kernel which are exposed for use by BPF programs. Unlike normal BPF helpers,
+kfuncs do not have a stable interface and can change from one kernel release to
+another. Hence, BPF programs need to be updated in response to changes in the
+kernel.
+
+2. Defining a kfunc
+===================
+
+There are two ways to expose a kernel function to BPF programs, either make an
+existing function in the kernel visible, or add a new wrapper for BPF. In both
+cases, care must be taken that BPF program can only call such function in a
+valid context. To enforce this, visibility of a kfunc can be per program type.
+
+If you are not creating a BPF wrapper for existing kernel function, skip ahead
+to :ref:`BPF_kfunc_nodef`.
+
+2.1 Creating a wrapper kfunc
+----------------------------
+
+When defining a wrapper kfunc, the wrapper function should have extern linkage.
+This prevents the compiler from optimizing away dead code, as this wrapper kfunc
+is not invoked anywhere in the kernel itself. It is not necessary to provide a
+prototype in a header for the wrapper kfunc.
+
+An example is given below::
+
+ /* Disables missing prototype warnings */
+ __diag_push();
+ __diag_ignore_all("-Wmissing-prototypes",
+ "Global kfuncs as their definitions will be in BTF");
+
+ struct task_struct *bpf_find_get_task_by_vpid(pid_t nr)
+ {
+ return find_get_task_by_vpid(nr);
+ }
+
+ __diag_pop();
+
+A wrapper kfunc is often needed when we need to annotate parameters of the
+kfunc. Otherwise one may directly make the kfunc visible to the BPF program by
+registering it with the BPF subsystem. See :ref:`BPF_kfunc_nodef`.
+
+2.2 Annotating kfunc parameters
+-------------------------------
+
+Similar to BPF helpers, there is sometime need for additional context required
+by the verifier to make the usage of kernel functions safer and more useful.
+Hence, we can annotate a parameter by suffixing the name of the argument of the
+kfunc with a __tag, where tag may be one of the supported annotations.
+
+2.2.1 __sz Annotation
+---------------------
+
+This annotation is used to indicate a memory and size pair in the argument list.
+An example is given below::
+
+ void bpf_memzero(void *mem, int mem__sz)
+ {
+ ...
+ }
+
+Here, the verifier will treat first argument as a PTR_TO_MEM, and second
+argument as its size. By default, without __sz annotation, the size of the type
+of the pointer is used. Without __sz annotation, a kfunc cannot accept a void
+pointer.
+
+.. _BPF_kfunc_nodef:
+
+2.3 Using an existing kernel function
+-------------------------------------
+
+When an existing function in the kernel is fit for consumption by BPF programs,
+it can be directly registered with the BPF subsystem. However, care must still
+be taken to review the context in which it will be invoked by the BPF program
+and whether it is safe to do so.
+
+2.4 Annotating kfuncs
+---------------------
+
+In addition to kfuncs' arguments, verifier may need more information about the
+type of kfunc(s) being registered with the BPF subsystem. To do so, we define
+flags on a set of kfuncs as follows::
+
+ BTF_SET8_START(bpf_task_set)
+ BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE | KF_RET_NULL)
+ BTF_ID_FLAGS(func, bpf_put_pid, KF_RELEASE)
+ BTF_SET8_END(bpf_task_set)
+
+This set encodes the BTF ID of each kfunc listed above, and encodes the flags
+along with it. Ofcourse, it is also allowed to specify no flags.
+
+2.4.1 KF_ACQUIRE flag
+---------------------
+
+The KF_ACQUIRE flag is used to indicate that the kfunc returns a pointer to a
+refcounted object. The verifier will then ensure that the pointer to the object
+is eventually released using a release kfunc, or transferred to a map using a
+referenced kptr (by invoking bpf_kptr_xchg). If not, the verifier fails the
+loading of the BPF program until no lingering references remain in all possible
+explored states of the program.
+
+2.4.2 KF_RET_NULL flag
+----------------------
+
+The KF_RET_NULL flag is used to indicate that the pointer returned by the kfunc
+may be NULL. Hence, it forces the user to do a NULL check on the pointer
+returned from the kfunc before making use of it (dereferencing or passing to
+another helper). This flag is often used in pairing with KF_ACQUIRE flag, but
+both are orthogonal to each other.
+
+2.4.3 KF_RELEASE flag
+---------------------
+
+The KF_RELEASE flag is used to indicate that the kfunc releases the pointer
+passed in to it. There can be only one referenced pointer that can be passed in.
+All copies of the pointer being released are invalidated as a result of invoking
+kfunc with this flag.
+
+2.4.4 KF_KPTR_GET flag
+----------------------
+
+The KF_KPTR_GET flag is used to indicate that the kfunc takes the first argument
+as a pointer to kptr, safely increments the refcount of the object it points to,
+and returns a reference to the user. The rest of the arguments may be normal
+arguments of a kfunc. The KF_KPTR_GET flag should be used in conjunction with
+KF_ACQUIRE and KF_RET_NULL flags.
+
+2.4.5 KF_TRUSTED_ARGS flag
+--------------------------
+
+The KF_TRUSTED_ARGS flag is used for kfuncs taking pointer arguments. It
+indicates that the all pointer arguments will always be refcounted, and have
+their offset set to 0. It can be used to enforce that a pointer to a refcounted
+object acquired from a kfunc or BPF helper is passed as an argument to this
+kfunc without any modifications (e.g. pointer arithmetic) such that it is
+trusted and points to the original object. This flag is often used for kfuncs
+that operate (change some property, perform some operation) on an object that
+was obtained using an acquire kfunc. Such kfuncs need an unchanged pointer to
+ensure the integrity of the operation being performed on the expected object.
+
+2.5 Registering the kfuncs
+--------------------------
+
+Once the kfunc is prepared for use, the final step to making it visible is
+registering it with the BPF subsystem. Registration is done per BPF program
+type. An example is shown below::
+
+ BTF_SET8_START(bpf_task_set)
+ BTF_ID_FLAGS(func, bpf_get_task_pid, KF_ACQUIRE | KF_RET_NULL)
+ BTF_ID_FLAGS(func, bpf_put_pid, KF_RELEASE)
+ BTF_SET8_END(bpf_task_set)
+
+ static const struct btf_kfunc_id_set bpf_task_kfunc_set = {
+ .owner = THIS_MODULE,
+ .set = &bpf_task_set,
+ };
+
+ static int init_subsystem(void)
+ {
+ return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_task_kfunc_set);
+ }
+ late_initcall(init_subsystem);
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 06/13] net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (4 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 05/13] bpf: Add documentation for kfuncs Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT Kumar Kartikeya Dwivedi
` (8 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
Move common checks inside the common function, and maintain the only
difference the two being how to obtain the struct net * from ctx.
No functional change intended.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
net/netfilter/nf_conntrack_bpf.c | 52 +++++++++++---------------------
1 file changed, 18 insertions(+), 34 deletions(-)
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index cf2096f65d0e..16304869264f 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -57,16 +57,19 @@ enum {
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, u8 *dir)
+ u32 tuple_len, struct bpf_ct_opts *opts,
+ u32 opts_len)
{
struct nf_conntrack_tuple_hash *hash;
struct nf_conntrack_tuple tuple;
struct nf_conn *ct;
- if (unlikely(protonum != IPPROTO_TCP && protonum != IPPROTO_UDP))
+ if (!opts || !bpf_tuple || opts->reserved[0] || opts->reserved[1] ||
+ opts_len != NF_BPF_CT_OPTS_SZ)
+ return ERR_PTR(-EINVAL);
+ if (unlikely(opts->l4proto != IPPROTO_TCP && opts->l4proto != IPPROTO_UDP))
return ERR_PTR(-EPROTO);
- if (unlikely(netns_id < BPF_F_CURRENT_NETNS))
+ if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
return ERR_PTR(-EINVAL);
memset(&tuple, 0, sizeof(tuple));
@@ -89,23 +92,22 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
return ERR_PTR(-EAFNOSUPPORT);
}
- tuple.dst.protonum = protonum;
+ tuple.dst.protonum = opts->l4proto;
- if (netns_id >= 0) {
- net = get_net_ns_by_id(net, netns_id);
+ if (opts->netns_id >= 0) {
+ net = get_net_ns_by_id(net, opts->netns_id);
if (unlikely(!net))
return ERR_PTR(-ENONET);
}
hash = nf_conntrack_find_get(net, &nf_ct_zone_dflt, &tuple);
- if (netns_id >= 0)
+ if (opts->netns_id >= 0)
put_net(net);
if (!hash)
return ERR_PTR(-ENOENT);
ct = nf_ct_tuplehash_to_ctrack(hash);
- if (dir)
- *dir = NF_CT_DIRECTION(hash);
+ opts->dir = NF_CT_DIRECTION(hash);
return ct;
}
@@ -138,20 +140,11 @@ bpf_xdp_ct_lookup(struct xdp_md *xdp_ctx, struct bpf_sock_tuple *bpf_tuple,
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__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, &opts->dir);
+ nfct = __bpf_nf_ct_lookup(caller_net, bpf_tuple, tuple__sz, opts, opts__sz);
if (IS_ERR(nfct)) {
- opts->error = PTR_ERR(nfct);
+ if (opts)
+ opts->error = PTR_ERR(nfct);
return NULL;
}
return nfct;
@@ -181,20 +174,11 @@ bpf_skb_ct_lookup(struct __sk_buff *skb_ctx, struct bpf_sock_tuple *bpf_tuple,
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__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, &opts->dir);
+ nfct = __bpf_nf_ct_lookup(caller_net, bpf_tuple, tuple__sz, opts, opts__sz);
if (IS_ERR(nfct)) {
- opts->error = PTR_ERR(nfct);
+ if (opts)
+ opts->error = PTR_ERR(nfct);
return NULL;
}
return nfct;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (5 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 06/13] net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-22 9:02 ` Pablo Neira Ayuso
2022-07-21 13:42 ` [PATCH bpf-next v7 08/13] net: netfilter: Add kfuncs to set and change CT timeout Kumar Kartikeya Dwivedi
` (7 subsequent siblings)
14 siblings, 1 reply; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev,
netfilter-devel
From: Lorenzo Bianconi <lorenzo@kernel.org>
Introduce bpf_xdp_ct_alloc, bpf_skb_ct_alloc and bpf_ct_insert_entry
kfuncs in order to insert a new entry from XDP and TC programs.
Introduce bpf_nf_ct_tuple_parse utility routine to consolidate common
code.
We extract out a helper __nf_ct_set_timeout, used by the ctnetlink and
nf_conntrack_bpf code, extract it out to nf_conntrack_core, so that
nf_conntrack_bpf doesn't need a dependency on CONFIG_NF_CT_NETLINK.
Later this helper will be reused as a helper to set timeout of allocated
but not yet inserted CT entry.
The allocation functions return struct nf_conn___init instead of
nf_conn, to distinguish allocated CT from an already inserted or looked
up CT. This is later used to enforce restrictions on what kfuncs
allocated CT can be used with.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Co-developed-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/net/netfilter/nf_conntrack_core.h | 15 ++
net/netfilter/nf_conntrack_bpf.c | 208 +++++++++++++++++++---
net/netfilter/nf_conntrack_netlink.c | 8 +-
3 files changed, 204 insertions(+), 27 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 37866c8386e2..83a60c684e6c 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -84,4 +84,19 @@ void nf_conntrack_lock(spinlock_t *lock);
extern spinlock_t nf_conntrack_expect_lock;
+/* ctnetlink code shared by both ctnetlink and nf_conntrack_bpf */
+
+#if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
+ (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) || \
+ IS_ENABLED(CONFIG_NF_CT_NETLINK))
+
+static inline void __nf_ct_set_timeout(struct nf_conn *ct, u64 timeout)
+{
+ if (timeout > INT_MAX)
+ timeout = INT_MAX;
+ WRITE_ONCE(ct->timeout, nfct_time_stamp + (u32)timeout);
+}
+
+#endif
+
#endif /* _NF_CONNTRACK_CORE_H */
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index 16304869264f..cac4a9558968 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -55,6 +55,94 @@ enum {
NF_BPF_CT_OPTS_SZ = 12,
};
+static int bpf_nf_ct_tuple_parse(struct bpf_sock_tuple *bpf_tuple,
+ u32 tuple_len, u8 protonum, u8 dir,
+ struct nf_conntrack_tuple *tuple)
+{
+ union nf_inet_addr *src = dir ? &tuple->dst.u3 : &tuple->src.u3;
+ union nf_inet_addr *dst = dir ? &tuple->src.u3 : &tuple->dst.u3;
+ union nf_conntrack_man_proto *sport = dir ? (void *)&tuple->dst.u
+ : &tuple->src.u;
+ union nf_conntrack_man_proto *dport = dir ? &tuple->src.u
+ : (void *)&tuple->dst.u;
+
+ if (unlikely(protonum != IPPROTO_TCP && protonum != IPPROTO_UDP))
+ return -EPROTO;
+
+ memset(tuple, 0, sizeof(*tuple));
+
+ switch (tuple_len) {
+ case sizeof(bpf_tuple->ipv4):
+ tuple->src.l3num = AF_INET;
+ src->ip = bpf_tuple->ipv4.saddr;
+ sport->tcp.port = bpf_tuple->ipv4.sport;
+ dst->ip = bpf_tuple->ipv4.daddr;
+ dport->tcp.port = bpf_tuple->ipv4.dport;
+ break;
+ case sizeof(bpf_tuple->ipv6):
+ tuple->src.l3num = AF_INET6;
+ memcpy(src->ip6, bpf_tuple->ipv6.saddr, sizeof(bpf_tuple->ipv6.saddr));
+ sport->tcp.port = bpf_tuple->ipv6.sport;
+ memcpy(dst->ip6, bpf_tuple->ipv6.daddr, sizeof(bpf_tuple->ipv6.daddr));
+ dport->tcp.port = bpf_tuple->ipv6.dport;
+ break;
+ default:
+ return -EAFNOSUPPORT;
+ }
+ tuple->dst.protonum = protonum;
+ tuple->dst.dir = dir;
+
+ return 0;
+}
+
+static struct nf_conn *
+__bpf_nf_ct_alloc_entry(struct net *net, struct bpf_sock_tuple *bpf_tuple,
+ u32 tuple_len, struct bpf_ct_opts *opts, u32 opts_len,
+ u32 timeout)
+{
+ struct nf_conntrack_tuple otuple, rtuple;
+ struct nf_conn *ct;
+ int err;
+
+ if (!opts || !bpf_tuple || opts->reserved[0] || opts->reserved[1] ||
+ opts_len != NF_BPF_CT_OPTS_SZ)
+ return ERR_PTR(-EINVAL);
+
+ if (unlikely(opts->netns_id < BPF_F_CURRENT_NETNS))
+ return ERR_PTR(-EINVAL);
+
+ err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
+ IP_CT_DIR_ORIGINAL, &otuple);
+ if (err < 0)
+ return ERR_PTR(err);
+
+ err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
+ IP_CT_DIR_REPLY, &rtuple);
+ if (err < 0)
+ return ERR_PTR(err);
+
+ if (opts->netns_id >= 0) {
+ net = get_net_ns_by_id(net, opts->netns_id);
+ if (unlikely(!net))
+ return ERR_PTR(-ENONET);
+ }
+
+ ct = nf_conntrack_alloc(net, &nf_ct_zone_dflt, &otuple, &rtuple,
+ GFP_ATOMIC);
+ if (IS_ERR(ct))
+ goto out;
+
+ memset(&ct->proto, 0, sizeof(ct->proto));
+ __nf_ct_set_timeout(ct, timeout * HZ);
+ ct->status |= IPS_CONFIRMED;
+
+out:
+ if (opts->netns_id >= 0)
+ put_net(net);
+
+ return ct;
+}
+
static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
struct bpf_sock_tuple *bpf_tuple,
u32 tuple_len, struct bpf_ct_opts *opts,
@@ -63,6 +151,7 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
struct nf_conntrack_tuple_hash *hash;
struct nf_conntrack_tuple tuple;
struct nf_conn *ct;
+ int err;
if (!opts || !bpf_tuple || opts->reserved[0] || opts->reserved[1] ||
opts_len != NF_BPF_CT_OPTS_SZ)
@@ -72,27 +161,10 @@ static struct nf_conn *__bpf_nf_ct_lookup(struct net *net,
if (unlikely(opts->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 = opts->l4proto;
+ err = bpf_nf_ct_tuple_parse(bpf_tuple, tuple_len, opts->l4proto,
+ IP_CT_DIR_ORIGINAL, &tuple);
+ if (err < 0)
+ return ERR_PTR(err);
if (opts->netns_id >= 0) {
net = get_net_ns_by_id(net, opts->netns_id);
@@ -116,6 +188,43 @@ __diag_push();
__diag_ignore_all("-Wmissing-prototypes",
"Global functions as their definitions will be in nf_conntrack BTF");
+struct nf_conn___init {
+ struct nf_conn ct;
+};
+
+/* bpf_xdp_ct_alloc - Allocate a new CT entry
+ *
+ * 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 allocation (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___init *
+bpf_xdp_ct_alloc(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 nf_conn *nfct;
+
+ nfct = __bpf_nf_ct_alloc_entry(dev_net(ctx->rxq->dev), bpf_tuple, tuple__sz,
+ opts, opts__sz, 10);
+ if (IS_ERR(nfct)) {
+ if (opts)
+ opts->error = PTR_ERR(nfct);
+ return NULL;
+ }
+
+ return (struct nf_conn___init *)nfct;
+}
+
/* bpf_xdp_ct_lookup - Lookup CT entry for the given tuple, and acquire a
* reference to it
*
@@ -150,6 +259,40 @@ bpf_xdp_ct_lookup(struct xdp_md *xdp_ctx, struct bpf_sock_tuple *bpf_tuple,
return nfct;
}
+/* bpf_skb_ct_alloc - Allocate a new CT entry
+ *
+ * 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 allocation (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___init *
+bpf_skb_ct_alloc(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 nf_conn *nfct;
+ struct net *net;
+
+ net = skb->dev ? dev_net(skb->dev) : sock_net(skb->sk);
+ nfct = __bpf_nf_ct_alloc_entry(net, bpf_tuple, tuple__sz, opts, opts__sz, 10);
+ if (IS_ERR(nfct)) {
+ if (opts)
+ opts->error = PTR_ERR(nfct);
+ return NULL;
+ }
+
+ return (struct nf_conn___init *)nfct;
+}
+
/* bpf_skb_ct_lookup - Lookup CT entry for the given tuple, and acquire a
* reference to it
*
@@ -184,6 +327,26 @@ bpf_skb_ct_lookup(struct __sk_buff *skb_ctx, struct bpf_sock_tuple *bpf_tuple,
return nfct;
}
+/* bpf_ct_insert_entry - Add the provided entry into a CT map
+ *
+ * This must be invoked for referenced PTR_TO_BTF_ID.
+ *
+ * @nfct__ref - Pointer to referenced nf_conn___init object, obtained
+ * using bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
+ */
+struct nf_conn *bpf_ct_insert_entry(struct nf_conn___init *nfct__ref)
+{
+ struct nf_conn *nfct = (struct nf_conn *)nfct__ref;
+ int err;
+
+ err = nf_conntrack_hash_check_insert(nfct);
+ if (err < 0) {
+ nf_conntrack_free(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
@@ -204,8 +367,11 @@ void bpf_ct_release(struct nf_conn *nfct)
__diag_pop()
BTF_SET8_START(nf_ct_kfunc_set)
+BTF_ID_FLAGS(func, bpf_xdp_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_xdp_ct_lookup, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_skb_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_skb_ct_lookup, KF_ACQUIRE | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_ct_insert_entry, KF_ACQUIRE | KF_RET_NULL | KF_RELEASE)
BTF_ID_FLAGS(func, bpf_ct_release, KF_RELEASE)
BTF_SET8_END(nf_ct_kfunc_set)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 722af5e309ba..0729b2f0d44f 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2025,9 +2025,7 @@ static int ctnetlink_change_timeout(struct nf_conn *ct,
{
u64 timeout = (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
- if (timeout > INT_MAX)
- timeout = INT_MAX;
- WRITE_ONCE(ct->timeout, nfct_time_stamp + (u32)timeout);
+ __nf_ct_set_timeout(ct, timeout);
if (test_bit(IPS_DYING_BIT, &ct->status))
return -ETIME;
@@ -2292,9 +2290,7 @@ ctnetlink_create_conntrack(struct net *net,
goto err1;
timeout = (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
- if (timeout > INT_MAX)
- timeout = INT_MAX;
- ct->timeout = (u32)timeout + nfct_time_stamp;
+ __nf_ct_set_timeout(ct, timeout);
rcu_read_lock();
if (cda[CTA_HELP]) {
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 08/13] net: netfilter: Add kfuncs to set and change CT timeout
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (6 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 09/13] net: netfilter: Add kfuncs to set and change CT status Kumar Kartikeya Dwivedi
` (6 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev,
netfilter-devel
Introduce bpf_ct_set_timeout and bpf_ct_change_timeout kfunc helpers in
order to change nf_conn timeout. This is same as ctnetlink_change_timeout,
hence code is shared between both by extracting it out to
__nf_ct_change_timeout. It is also updated to return an error when it
sees IPS_FIXED_TIMEOUT_BIT bit in ct->status, as that check was missing.
It is required to introduce two kfuncs taking nf_conn___init and nf_conn
instead of sharing one because KF_TRUSTED_ARGS flag causes strict type
checking. This would disallow passing nf_conn___init to kfunc taking
nf_conn, and vice versa. We cannot remove the KF_TRUSTED_ARGS flag as we
only want to accept refcounted pointers and not e.g. ct->master.
Apart from this, bpf_ct_set_timeout is only called for newly allocated
CT so it doesn't need to inspect the status field just yet. Sharing the
helpers even if it was possible would make timeout setting helper
sensitive to order of setting status and timeout after allocation.
Hence, bpf_ct_set_* kfuncs are meant to be used on allocated CT, and
bpf_ct_change_* kfuncs are meant to be used on inserted or looked up
CT entry.
Co-developed-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/net/netfilter/nf_conntrack_core.h | 2 ++
net/netfilter/nf_conntrack_bpf.c | 38 +++++++++++++++++++++--
net/netfilter/nf_conntrack_core.c | 22 +++++++++++++
net/netfilter/nf_conntrack_netlink.c | 9 +-----
4 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 83a60c684e6c..3b0f7d0eebae 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -97,6 +97,8 @@ static inline void __nf_ct_set_timeout(struct nf_conn *ct, u64 timeout)
WRITE_ONCE(ct->timeout, nfct_time_stamp + (u32)timeout);
}
+int __nf_ct_change_timeout(struct nf_conn *ct, u64 cta_timeout);
+
#endif
#endif /* _NF_CONNTRACK_CORE_H */
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index cac4a9558968..b8912e15082f 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -331,12 +331,12 @@ bpf_skb_ct_lookup(struct __sk_buff *skb_ctx, struct bpf_sock_tuple *bpf_tuple,
*
* This must be invoked for referenced PTR_TO_BTF_ID.
*
- * @nfct__ref - Pointer to referenced nf_conn___init object, obtained
+ * @nfct - Pointer to referenced nf_conn___init object, obtained
* using bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
*/
-struct nf_conn *bpf_ct_insert_entry(struct nf_conn___init *nfct__ref)
+struct nf_conn *bpf_ct_insert_entry(struct nf_conn___init *nfct_i)
{
- struct nf_conn *nfct = (struct nf_conn *)nfct__ref;
+ struct nf_conn *nfct = (struct nf_conn *)nfct_i;
int err;
err = nf_conntrack_hash_check_insert(nfct);
@@ -364,6 +364,36 @@ void bpf_ct_release(struct nf_conn *nfct)
nf_ct_put(nfct);
}
+/* bpf_ct_set_timeout - Set timeout of allocated nf_conn
+ *
+ * Sets the default timeout of newly allocated nf_conn before insertion.
+ * This helper must be invoked for refcounted pointer to nf_conn___init.
+ *
+ * Parameters:
+ * @nfct - Pointer to referenced nf_conn object, obtained using
+ * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
+ * @timeout - Timeout in msecs.
+ */
+void bpf_ct_set_timeout(struct nf_conn___init *nfct, u32 timeout)
+{
+ __nf_ct_set_timeout((struct nf_conn *)nfct, msecs_to_jiffies(timeout));
+}
+
+/* bpf_ct_change_timeout - Change timeout of inserted nf_conn
+ *
+ * Change timeout associated of the inserted or looked up nf_conn.
+ * This helper must be invoked for refcounted pointer to nf_conn.
+ *
+ * Parameters:
+ * @nfct - Pointer to referenced nf_conn object, obtained using
+ * bpf_ct_insert_entry, bpf_xdp_ct_lookup, or bpf_skb_ct_lookup.
+ * @timeout - New timeout in msecs.
+ */
+int bpf_ct_change_timeout(struct nf_conn *nfct, u32 timeout)
+{
+ return __nf_ct_change_timeout(nfct, msecs_to_jiffies(timeout));
+}
+
__diag_pop()
BTF_SET8_START(nf_ct_kfunc_set)
@@ -373,6 +403,8 @@ BTF_ID_FLAGS(func, bpf_skb_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_skb_ct_lookup, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_ct_insert_entry, KF_ACQUIRE | KF_RET_NULL | KF_RELEASE)
BTF_ID_FLAGS(func, bpf_ct_release, KF_RELEASE)
+BTF_ID_FLAGS(func, bpf_ct_set_timeout, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_ct_change_timeout, KF_TRUSTED_ARGS)
BTF_SET8_END(nf_ct_kfunc_set)
static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 082a2fd8d85b..572f59a5e936 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2786,3 +2786,25 @@ int nf_conntrack_init_net(struct net *net)
free_percpu(net->ct.stat);
return ret;
}
+
+#if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
+ (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) || \
+ IS_ENABLED(CONFIG_NF_CT_NETLINK))
+
+/* ctnetlink code shared by both ctnetlink and nf_conntrack_bpf */
+
+int __nf_ct_change_timeout(struct nf_conn *ct, u64 timeout)
+{
+ if (test_bit(IPS_FIXED_TIMEOUT_BIT, &ct->status))
+ return -EPERM;
+
+ __nf_ct_set_timeout(ct, timeout);
+
+ if (test_bit(IPS_DYING_BIT, &ct->status))
+ return -ETIME;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(__nf_ct_change_timeout);
+
+#endif
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 0729b2f0d44f..b1de07c73845 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2023,14 +2023,7 @@ static int ctnetlink_change_helper(struct nf_conn *ct,
static int ctnetlink_change_timeout(struct nf_conn *ct,
const struct nlattr * const cda[])
{
- u64 timeout = (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ;
-
- __nf_ct_set_timeout(ct, timeout);
-
- if (test_bit(IPS_DYING_BIT, &ct->status))
- return -ETIME;
-
- return 0;
+ return __nf_ct_change_timeout(ct, (u64)ntohl(nla_get_be32(cda[CTA_TIMEOUT])) * HZ);
}
#if defined(CONFIG_NF_CONNTRACK_MARK)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 09/13] net: netfilter: Add kfuncs to set and change CT status
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (7 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 08/13] net: netfilter: Add kfuncs to set and change CT timeout Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 10/13] selftests/bpf: Add verifier tests for trusted kfunc args Kumar Kartikeya Dwivedi
` (5 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev,
netfilter-devel
From: Lorenzo Bianconi <lorenzo@kernel.org>
Introduce bpf_ct_set_status and bpf_ct_change_status kfunc helpers in
order to set nf_conn field of allocated entry or update nf_conn status
field of existing inserted entry. Use nf_ct_change_status_common to
share the permitted status field changes between netlink and BPF side
by refactoring ctnetlink_change_status.
It is required to introduce two kfuncs taking nf_conn___init and nf_conn
instead of sharing one because KF_TRUSTED_ARGS flag causes strict type
checking. This would disallow passing nf_conn___init to kfunc taking
nf_conn, and vice versa. We cannot remove the KF_TRUSTED_ARGS flag as we
only want to accept refcounted pointers and not e.g. ct->master.
Hence, bpf_ct_set_* kfuncs are meant to be used on allocated CT, and
bpf_ct_change_* kfuncs are meant to be used on inserted or looked up
CT entry.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Co-developed-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/net/netfilter/nf_conntrack_core.h | 2 ++
net/netfilter/nf_conntrack_bpf.c | 32 ++++++++++++++++++
net/netfilter/nf_conntrack_core.c | 40 +++++++++++++++++++++++
net/netfilter/nf_conntrack_netlink.c | 39 ++--------------------
4 files changed, 76 insertions(+), 37 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 3b0f7d0eebae..3cd3a6e631aa 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -98,6 +98,8 @@ static inline void __nf_ct_set_timeout(struct nf_conn *ct, u64 timeout)
}
int __nf_ct_change_timeout(struct nf_conn *ct, u64 cta_timeout);
+void __nf_ct_change_status(struct nf_conn *ct, unsigned long on, unsigned long off);
+int nf_ct_change_status_common(struct nf_conn *ct, unsigned int status);
#endif
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index b8912e15082f..1cd87b28c9b0 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -394,6 +394,36 @@ int bpf_ct_change_timeout(struct nf_conn *nfct, u32 timeout)
return __nf_ct_change_timeout(nfct, msecs_to_jiffies(timeout));
}
+/* bpf_ct_set_status - Set status field of allocated nf_conn
+ *
+ * Set the status field of the newly allocated nf_conn before insertion.
+ * This must be invoked for referenced PTR_TO_BTF_ID to nf_conn___init.
+ *
+ * Parameters:
+ * @nfct - Pointer to referenced nf_conn object, obtained using
+ * bpf_xdp_ct_alloc or bpf_skb_ct_alloc.
+ * @status - New status value.
+ */
+int bpf_ct_set_status(const struct nf_conn___init *nfct, u32 status)
+{
+ return nf_ct_change_status_common((struct nf_conn *)nfct, status);
+}
+
+/* bpf_ct_change_status - Change status of inserted nf_conn
+ *
+ * Change the status field of the provided connection tracking entry.
+ * This must be invoked for referenced PTR_TO_BTF_ID to nf_conn.
+ *
+ * Parameters:
+ * @nfct - Pointer to referenced nf_conn object, obtained using
+ * bpf_ct_insert_entry, bpf_xdp_ct_lookup or bpf_skb_ct_lookup.
+ * @status - New status value.
+ */
+int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
+{
+ return nf_ct_change_status_common(nfct, status);
+}
+
__diag_pop()
BTF_SET8_START(nf_ct_kfunc_set)
@@ -405,6 +435,8 @@ BTF_ID_FLAGS(func, bpf_ct_insert_entry, KF_ACQUIRE | KF_RET_NULL | KF_RELEASE)
BTF_ID_FLAGS(func, bpf_ct_release, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_ct_set_timeout, KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_ct_change_timeout, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_ct_set_status, KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_ct_change_status, KF_TRUSTED_ARGS)
BTF_SET8_END(nf_ct_kfunc_set)
static const struct btf_kfunc_id_set nf_conntrack_kfunc_set = {
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 572f59a5e936..66a0aa8dbc3b 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -2807,4 +2807,44 @@ int __nf_ct_change_timeout(struct nf_conn *ct, u64 timeout)
}
EXPORT_SYMBOL_GPL(__nf_ct_change_timeout);
+void __nf_ct_change_status(struct nf_conn *ct, unsigned long on, unsigned long off)
+{
+ unsigned int bit;
+
+ /* Ignore these unchangable bits */
+ on &= ~IPS_UNCHANGEABLE_MASK;
+ off &= ~IPS_UNCHANGEABLE_MASK;
+
+ for (bit = 0; bit < __IPS_MAX_BIT; bit++) {
+ if (on & (1 << bit))
+ set_bit(bit, &ct->status);
+ else if (off & (1 << bit))
+ clear_bit(bit, &ct->status);
+ }
+}
+EXPORT_SYMBOL_GPL(__nf_ct_change_status);
+
+int nf_ct_change_status_common(struct nf_conn *ct, unsigned int status)
+{
+ unsigned long d;
+
+ d = ct->status ^ status;
+
+ if (d & (IPS_EXPECTED|IPS_CONFIRMED|IPS_DYING))
+ /* unchangeable */
+ return -EBUSY;
+
+ if (d & IPS_SEEN_REPLY && !(status & IPS_SEEN_REPLY))
+ /* SEEN_REPLY bit can only be set */
+ return -EBUSY;
+
+ if (d & IPS_ASSURED && !(status & IPS_ASSURED))
+ /* ASSURED bit can only be set */
+ return -EBUSY;
+
+ __nf_ct_change_status(ct, status, 0);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nf_ct_change_status_common);
+
#endif
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index b1de07c73845..e02832ef9b9f 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1890,45 +1890,10 @@ ctnetlink_parse_nat_setup(struct nf_conn *ct,
}
#endif
-static void
-__ctnetlink_change_status(struct nf_conn *ct, unsigned long on,
- unsigned long off)
-{
- unsigned int bit;
-
- /* Ignore these unchangable bits */
- on &= ~IPS_UNCHANGEABLE_MASK;
- off &= ~IPS_UNCHANGEABLE_MASK;
-
- for (bit = 0; bit < __IPS_MAX_BIT; bit++) {
- if (on & (1 << bit))
- set_bit(bit, &ct->status);
- else if (off & (1 << bit))
- clear_bit(bit, &ct->status);
- }
-}
-
static int
ctnetlink_change_status(struct nf_conn *ct, const struct nlattr * const cda[])
{
- unsigned long d;
- unsigned int status = ntohl(nla_get_be32(cda[CTA_STATUS]));
- d = ct->status ^ status;
-
- if (d & (IPS_EXPECTED|IPS_CONFIRMED|IPS_DYING))
- /* unchangeable */
- return -EBUSY;
-
- if (d & IPS_SEEN_REPLY && !(status & IPS_SEEN_REPLY))
- /* SEEN_REPLY bit can only be set */
- return -EBUSY;
-
- if (d & IPS_ASSURED && !(status & IPS_ASSURED))
- /* ASSURED bit can only be set */
- return -EBUSY;
-
- __ctnetlink_change_status(ct, status, 0);
- return 0;
+ return nf_ct_change_status_common(ct, ntohl(nla_get_be32(cda[CTA_STATUS])));
}
static int
@@ -2825,7 +2790,7 @@ ctnetlink_update_status(struct nf_conn *ct, const struct nlattr * const cda[])
* unchangeable bits but do not error out. Also user programs
* are allowed to clear the bits that they are allowed to change.
*/
- __ctnetlink_change_status(ct, status, ~status);
+ __nf_ct_change_status(ct, status, ~status);
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 10/13] selftests/bpf: Add verifier tests for trusted kfunc args
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (8 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 09/13] net: netfilter: Add kfuncs to set and change CT status Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 11/13] selftests/bpf: Add tests for new nf_conntrack kfuncs Kumar Kartikeya Dwivedi
` (4 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Yonghong Song, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen,
Lorenzo Bianconi, netdev, netfilter-devel
Make sure verifier rejects the bad cases and ensure the good case keeps
working. The selftests make use of the bpf_kfunc_call_test_ref kfunc
added in the previous patch only for verification.
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
tools/testing/selftests/bpf/verifier/calls.c | 53 ++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index 743ed34c1238..3fb4f69b1962 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -218,6 +218,59 @@
.result = REJECT,
.errstr = "variable ptr_ access var_off=(0x0; 0x7) disallowed",
},
+{
+ "calls: invalid kfunc call: referenced arg needs refcounted PTR_TO_BTF_ID",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_6, 16),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .fixup_kfunc_btf_id = {
+ { "bpf_kfunc_call_test_acquire", 3 },
+ { "bpf_kfunc_call_test_ref", 8 },
+ { "bpf_kfunc_call_test_ref", 10 },
+ },
+ .result_unpriv = REJECT,
+ .result = REJECT,
+ .errstr = "R1 must be referenced",
+},
+{
+ "calls: valid kfunc call: referenced arg needs refcounted PTR_TO_BTF_ID",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .fixup_kfunc_btf_id = {
+ { "bpf_kfunc_call_test_acquire", 3 },
+ { "bpf_kfunc_call_test_ref", 8 },
+ { "bpf_kfunc_call_test_release", 10 },
+ },
+ .result_unpriv = REJECT,
+ .result = ACCEPT,
+},
{
"calls: basic sanity",
.insns = {
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 11/13] selftests/bpf: Add tests for new nf_conntrack kfuncs
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (9 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 10/13] selftests/bpf: Add verifier tests for trusted kfunc args Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 12/13] selftests/bpf: Add negative " Kumar Kartikeya Dwivedi
` (3 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Pablo Neira Ayuso, Florian Westphal,
Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev,
netfilter-devel
From: Lorenzo Bianconi <lorenzo@kernel.org>
Introduce selftests for the following kfunc helpers:
- bpf_xdp_ct_alloc
- bpf_skb_ct_alloc
- bpf_ct_insert_entry
- bpf_ct_set_timeout
- bpf_ct_change_timeout
- bpf_ct_set_status
- bpf_ct_change_status
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
.../testing/selftests/bpf/prog_tests/bpf_nf.c | 8 ++
.../testing/selftests/bpf/progs/test_bpf_nf.c | 85 ++++++++++++++++---
2 files changed, 81 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
index dd30b1e3a67c..cbada73a61f8 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
@@ -39,6 +39,14 @@ void test_bpf_nf_ct(int mode)
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");
+ ASSERT_EQ(skel->data->test_alloc_entry, 0, "Test for alloc new entry");
+ ASSERT_EQ(skel->data->test_insert_entry, 0, "Test for insert new entry");
+ ASSERT_EQ(skel->data->test_succ_lookup, 0, "Test for successful lookup");
+ /* allow some tolerance for test_delta_timeout value to avoid races. */
+ ASSERT_GT(skel->bss->test_delta_timeout, 8, "Test for min ct timeout update");
+ ASSERT_LE(skel->bss->test_delta_timeout, 10, "Test for max ct timeout update");
+ /* expected status is IPS_SEEN_REPLY */
+ ASSERT_EQ(skel->bss->test_status, 2, "Test for ct status update ");
end:
test_bpf_nf__destroy(skel);
}
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
index f00a9731930e..196cd8dfe42a 100644
--- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c
+++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
@@ -8,6 +8,8 @@
#define EINVAL 22
#define ENOENT 2
+extern unsigned long CONFIG_HZ __kconfig;
+
int test_einval_bpf_tuple = 0;
int test_einval_reserved = 0;
int test_einval_netns_id = 0;
@@ -16,6 +18,11 @@ int test_eproto_l4proto = 0;
int test_enonet_netns_id = 0;
int test_enoent_lookup = 0;
int test_eafnosupport = 0;
+int test_alloc_entry = -EINVAL;
+int test_insert_entry = -EAFNOSUPPORT;
+int test_succ_lookup = -ENOENT;
+u32 test_delta_timeout = 0;
+u32 test_status = 0;
struct nf_conn;
@@ -26,31 +33,44 @@ struct bpf_ct_opts___local {
u8 reserved[3];
} __attribute__((preserve_access_index));
+struct nf_conn *bpf_xdp_ct_alloc(struct xdp_md *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts___local *, u32) __ksym;
struct nf_conn *bpf_xdp_ct_lookup(struct xdp_md *, struct bpf_sock_tuple *, u32,
struct bpf_ct_opts___local *, u32) __ksym;
+struct nf_conn *bpf_skb_ct_alloc(struct __sk_buff *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts___local *, u32) __ksym;
struct nf_conn *bpf_skb_ct_lookup(struct __sk_buff *, struct bpf_sock_tuple *, u32,
struct bpf_ct_opts___local *, u32) __ksym;
+struct nf_conn *bpf_ct_insert_entry(struct nf_conn *) __ksym;
void bpf_ct_release(struct nf_conn *) __ksym;
+void bpf_ct_set_timeout(struct nf_conn *, u32) __ksym;
+int bpf_ct_change_timeout(struct nf_conn *, u32) __ksym;
+int bpf_ct_set_status(struct nf_conn *, u32) __ksym;
+int bpf_ct_change_status(struct nf_conn *, u32) __ksym;
static __always_inline void
-nf_ct_test(struct nf_conn *(*func)(void *, struct bpf_sock_tuple *, u32,
- struct bpf_ct_opts___local *, u32),
+nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts___local *, u32),
+ struct nf_conn *(*alloc_fn)(void *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts___local *, u32),
void *ctx)
{
struct bpf_ct_opts___local opts_def = { .l4proto = IPPROTO_TCP, .netns_id = -1 };
struct bpf_sock_tuple bpf_tuple;
struct nf_conn *ct;
+ int err;
__builtin_memset(&bpf_tuple, 0, sizeof(bpf_tuple.ipv4));
- ct = func(ctx, NULL, 0, &opts_def, sizeof(opts_def));
+ ct = lookup_fn(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));
+ ct = lookup_fn(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def,
+ sizeof(opts_def));
opts_def.reserved[0] = 0;
opts_def.l4proto = IPPROTO_TCP;
if (ct)
@@ -59,21 +79,24 @@ nf_ct_test(struct nf_conn *(*func)(void *, struct bpf_sock_tuple *, u32,
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));
+ ct = lookup_fn(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);
+ ct = lookup_fn(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));
+ ct = lookup_fn(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def,
+ sizeof(opts_def));
opts_def.l4proto = IPPROTO_TCP;
if (ct)
bpf_ct_release(ct);
@@ -81,37 +104,75 @@ nf_ct_test(struct nf_conn *(*func)(void *, struct bpf_sock_tuple *, u32,
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));
+ ct = lookup_fn(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));
+ ct = lookup_fn(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));
+ ct = lookup_fn(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;
+
+ bpf_tuple.ipv4.saddr = bpf_get_prandom_u32(); /* src IP */
+ bpf_tuple.ipv4.daddr = bpf_get_prandom_u32(); /* dst IP */
+ bpf_tuple.ipv4.sport = bpf_get_prandom_u32(); /* src port */
+ bpf_tuple.ipv4.dport = bpf_get_prandom_u32(); /* dst port */
+
+ ct = alloc_fn(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4), &opts_def,
+ sizeof(opts_def));
+ if (ct) {
+ struct nf_conn *ct_ins;
+
+ bpf_ct_set_timeout(ct, 10000);
+ bpf_ct_set_status(ct, IPS_CONFIRMED);
+
+ ct_ins = bpf_ct_insert_entry(ct);
+ if (ct_ins) {
+ struct nf_conn *ct_lk;
+
+ ct_lk = lookup_fn(ctx, &bpf_tuple, sizeof(bpf_tuple.ipv4),
+ &opts_def, sizeof(opts_def));
+ if (ct_lk) {
+ /* update ct entry timeout */
+ bpf_ct_change_timeout(ct_lk, 10000);
+ test_delta_timeout = ct_lk->timeout - bpf_jiffies64();
+ test_delta_timeout /= CONFIG_HZ;
+ test_status = IPS_SEEN_REPLY;
+ bpf_ct_change_status(ct_lk, IPS_SEEN_REPLY);
+ bpf_ct_release(ct_lk);
+ test_succ_lookup = 0;
+ }
+ bpf_ct_release(ct_ins);
+ test_insert_entry = 0;
+ }
+ test_alloc_entry = 0;
+ }
}
SEC("xdp")
int nf_xdp_ct_test(struct xdp_md *ctx)
{
- nf_ct_test((void *)bpf_xdp_ct_lookup, ctx);
+ nf_ct_test((void *)bpf_xdp_ct_lookup, (void *)bpf_xdp_ct_alloc, ctx);
return 0;
}
SEC("tc")
int nf_skb_ct_test(struct __sk_buff *ctx)
{
- nf_ct_test((void *)bpf_skb_ct_lookup, ctx);
+ nf_ct_test((void *)bpf_skb_ct_lookup, (void *)bpf_skb_ct_alloc, ctx);
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 12/13] selftests/bpf: Add negative tests for new nf_conntrack kfuncs
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (10 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 11/13] selftests/bpf: Add tests for new nf_conntrack kfuncs Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 13/13] selftests/bpf: Fix test_verifier failed test in unprivileged mode Kumar Kartikeya Dwivedi
` (2 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
Test cases we care about and ensure improper usage is caught and
rejected by the verifier.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
.../testing/selftests/bpf/prog_tests/bpf_nf.c | 56 +++++++-
.../selftests/bpf/progs/test_bpf_nf_fail.c | 134 ++++++++++++++++++
2 files changed, 189 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
index cbada73a61f8..7a74a1579076 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
@@ -2,13 +2,29 @@
#include <test_progs.h>
#include <network_helpers.h>
#include "test_bpf_nf.skel.h"
+#include "test_bpf_nf_fail.skel.h"
+
+static char log_buf[1024 * 1024];
+
+struct {
+ const char *prog_name;
+ const char *err_msg;
+} test_bpf_nf_fail_tests[] = {
+ { "alloc_release", "kernel function bpf_ct_release args#0 expected pointer to STRUCT nf_conn but" },
+ { "insert_insert", "kernel function bpf_ct_insert_entry args#0 expected pointer to STRUCT nf_conn___init but" },
+ { "lookup_insert", "kernel function bpf_ct_insert_entry args#0 expected pointer to STRUCT nf_conn___init but" },
+ { "set_timeout_after_insert", "kernel function bpf_ct_set_timeout args#0 expected pointer to STRUCT nf_conn___init but" },
+ { "set_status_after_insert", "kernel function bpf_ct_set_status args#0 expected pointer to STRUCT nf_conn___init but" },
+ { "change_timeout_after_alloc", "kernel function bpf_ct_change_timeout args#0 expected pointer to STRUCT nf_conn but" },
+ { "change_status_after_alloc", "kernel function bpf_ct_change_status args#0 expected pointer to STRUCT nf_conn but" },
+};
enum {
TEST_XDP,
TEST_TC_BPF,
};
-void test_bpf_nf_ct(int mode)
+static void test_bpf_nf_ct(int mode)
{
struct test_bpf_nf *skel;
int prog_fd, err;
@@ -51,10 +67,48 @@ void test_bpf_nf_ct(int mode)
test_bpf_nf__destroy(skel);
}
+static void test_bpf_nf_ct_fail(const char *prog_name, const char *err_msg)
+{
+ LIBBPF_OPTS(bpf_object_open_opts, opts, .kernel_log_buf = log_buf,
+ .kernel_log_size = sizeof(log_buf),
+ .kernel_log_level = 1);
+ struct test_bpf_nf_fail *skel;
+ struct bpf_program *prog;
+ int ret;
+
+ skel = test_bpf_nf_fail__open_opts(&opts);
+ if (!ASSERT_OK_PTR(skel, "test_bpf_nf_fail__open"))
+ return;
+
+ prog = bpf_object__find_program_by_name(skel->obj, prog_name);
+ if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name"))
+ goto end;
+
+ bpf_program__set_autoload(prog, true);
+
+ ret = test_bpf_nf_fail__load(skel);
+ if (!ASSERT_ERR(ret, "test_bpf_nf_fail__load must fail"))
+ goto end;
+
+ if (!ASSERT_OK_PTR(strstr(log_buf, err_msg), "expected error message")) {
+ fprintf(stderr, "Expected: %s\n", err_msg);
+ fprintf(stderr, "Verifier: %s\n", log_buf);
+ }
+
+end:
+ test_bpf_nf_fail__destroy(skel);
+}
+
void test_bpf_nf(void)
{
+ int i;
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);
+ for (i = 0; i < ARRAY_SIZE(test_bpf_nf_fail_tests); i++) {
+ if (test__start_subtest(test_bpf_nf_fail_tests[i].prog_name))
+ test_bpf_nf_ct_fail(test_bpf_nf_fail_tests[i].prog_name,
+ test_bpf_nf_fail_tests[i].err_msg);
+ }
}
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
new file mode 100644
index 000000000000..bf79af15c808
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+
+struct nf_conn;
+
+struct bpf_ct_opts___local {
+ s32 netns_id;
+ s32 error;
+ u8 l4proto;
+ u8 reserved[3];
+} __attribute__((preserve_access_index));
+
+struct nf_conn *bpf_skb_ct_alloc(struct __sk_buff *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts___local *, u32) __ksym;
+struct nf_conn *bpf_skb_ct_lookup(struct __sk_buff *, struct bpf_sock_tuple *, u32,
+ struct bpf_ct_opts___local *, u32) __ksym;
+struct nf_conn *bpf_ct_insert_entry(struct nf_conn *) __ksym;
+void bpf_ct_release(struct nf_conn *) __ksym;
+void bpf_ct_set_timeout(struct nf_conn *, u32) __ksym;
+int bpf_ct_change_timeout(struct nf_conn *, u32) __ksym;
+int bpf_ct_set_status(struct nf_conn *, u32) __ksym;
+int bpf_ct_change_status(struct nf_conn *, u32) __ksym;
+
+SEC("?tc")
+int alloc_release(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_alloc(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ bpf_ct_release(ct);
+ return 0;
+}
+
+SEC("?tc")
+int insert_insert(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_alloc(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ ct = bpf_ct_insert_entry(ct);
+ if (!ct)
+ return 0;
+ ct = bpf_ct_insert_entry(ct);
+ return 0;
+}
+
+SEC("?tc")
+int lookup_insert(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_lookup(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ bpf_ct_insert_entry(ct);
+ return 0;
+}
+
+SEC("?tc")
+int set_timeout_after_insert(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_alloc(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ ct = bpf_ct_insert_entry(ct);
+ if (!ct)
+ return 0;
+ bpf_ct_set_timeout(ct, 0);
+ return 0;
+}
+
+SEC("?tc")
+int set_status_after_insert(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_alloc(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ ct = bpf_ct_insert_entry(ct);
+ if (!ct)
+ return 0;
+ bpf_ct_set_status(ct, 0);
+ return 0;
+}
+
+SEC("?tc")
+int change_timeout_after_alloc(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_alloc(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ bpf_ct_change_timeout(ct, 0);
+ return 0;
+}
+
+SEC("?tc")
+int change_status_after_alloc(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_alloc(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ bpf_ct_change_status(ct, 0);
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v7 13/13] selftests/bpf: Fix test_verifier failed test in unprivileged mode
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (11 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 12/13] selftests/bpf: Add negative " Kumar Kartikeya Dwivedi
@ 2022-07-21 13:42 ` Kumar Kartikeya Dwivedi
2022-07-21 17:28 ` [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Zvi Effron
2022-07-22 4:10 ` patchwork-bot+netdevbpf
14 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 13:42 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
Loading the BTF won't be permitted without privileges, hence only test
for privileged mode by setting the prog type. This makes the
test_verifier show 0 failures when unprivileged BPF is enabled.
Fixes: 4118e9e9def ("selftest/bpf: Test for use-after-free bug fix in inline_bpf_loop")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
tools/testing/selftests/bpf/verifier/bpf_loop_inline.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c b/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c
index 2d0023659d88..a535d41dc20d 100644
--- a/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c
+++ b/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c
@@ -251,6 +251,7 @@
.expected_insns = { PSEUDO_CALL_INSN() },
.unexpected_insns = { HELPER_CALL_INSN() },
.result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_TRACEPOINT,
.func_info = { { 0, MAIN_TYPE }, { 16, CALLBACK_TYPE } },
.func_info_cnt = 2,
BTF_TYPES
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (12 preceding siblings ...)
2022-07-21 13:42 ` [PATCH bpf-next v7 13/13] selftests/bpf: Fix test_verifier failed test in unprivileged mode Kumar Kartikeya Dwivedi
@ 2022-07-21 17:28 ` Zvi Effron
2022-07-21 18:01 ` Kumar Kartikeya Dwivedi
2022-07-22 4:10 ` patchwork-bot+netdevbpf
14 siblings, 1 reply; 29+ messages in thread
From: Zvi Effron @ 2022-07-21 17:28 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
On Thu, Jul 21, 2022 at 6:43 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> Introduce the following new kfuncs:
> - bpf_{xdp,skb}_ct_alloc
> - bpf_ct_insert_entry
> - bpf_ct_{set,change}_timeout
> - bpf_ct_{set,change}_status
>
> The setting of timeout and status on allocated or inserted/looked up CT
> is same as the ctnetlink interface, hence code is refactored and shared
> with the kfuncs. It is ensured allocated CT cannot be passed to kfuncs
> that expected inserted CT, and vice versa. Please see individual patches
> for details.
>
Is it expected that using these helpers and the kernel's conntrack to manage
connection state from XDP will outperform using maps and eBPF timers (for XDP
use cases that don't have a userspace component that also needs the information
in conntrack)? Have you done any benchmarking on the performance of using
conntrack from XDP?
Thanks!
--Zvi
> Changelog:
> ----------
> v6 -> v7:
> v6: https://lore.kernel.org/bpf/20220719132430.19993-1-memxor@gmail.com
>
> * Use .long to encode flags (Alexei)
> * Fix description of KF_RET_NULL in documentation (Toke)
>
> v5 -> v6:
> v5: https://lore.kernel.org/bpf/20220623192637.3866852-1-memxor@gmail.com
>
> * Introduce kfunc flags, rework verifier to work with them
> * Add documentation for kfuncs
> * Add comment explaining TRUSTED_ARGS kfunc flag (Alexei)
> * Fix missing offset check for trusted arguments (Alexei)
> * Change nf_conntrack test minimum delta value to 8
>
> v4 -> v5:
> v4: https://lore.kernel.org/bpf/cover.1653600577.git.lorenzo@kernel.org
>
> * Drop read-only PTR_TO_BTF_ID approach, use struct nf_conn___init (Alexei)
> * Drop acquire release pair code that is no longer required (Alexei)
> * Disable writes into nf_conn, use dedicated helpers (Florian, Alexei)
> * Refactor and share ctnetlink code for setting timeout and status
> * Do strict type matching on finding __ref suffix on argument to
> prevent passing nf_conn___init as nf_conn (offset = 0, match on walk)
> * Remove bpf_ct_opts parameter from bpf_ct_insert_entry
> * Update selftests for new additions, add more negative tests
>
> v3 -> v4:
> v3: https://lore.kernel.org/bpf/cover.1652870182.git.lorenzo@kernel.org
>
> * split bpf_xdp_ct_add in bpf_xdp_ct_alloc/bpf_skb_ct_alloc and
> bpf_ct_insert_entry
> * add verifier code to properly populate/configure ct entry
> * improve selftests
>
> v2 -> v3:
> v2: https://lore.kernel.org/bpf/cover.1652372970.git.lorenzo@kernel.org
>
> * add bpf_xdp_ct_add and bpf_ct_refresh_timeout kfunc helpers
> * remove conntrack dependency from selftests
> * add support for forcing kfunc args to be referenced and related selftests
>
> v1 -> v2:
> v1: https://lore.kernel.org/bpf/1327f8f5696ff2bc60400e8f3b79047914ccc837.1651595019.git.lorenzo@kernel.org
>
> * add bpf_ct_refresh_timeout kfunc selftest
>
> Kumar Kartikeya Dwivedi (10):
> bpf: Introduce 8-byte BTF set
> tools/resolve_btfids: Add support for 8-byte BTF sets
> bpf: Switch to new kfunc flags infrastructure
> bpf: Add support for forcing kfunc args to be trusted
> bpf: Add documentation for kfuncs
> net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup
> net: netfilter: Add kfuncs to set and change CT timeout
> selftests/bpf: Add verifier tests for trusted kfunc args
> selftests/bpf: Add negative tests for new nf_conntrack kfuncs
> selftests/bpf: Fix test_verifier failed test in unprivileged mode
>
> Lorenzo Bianconi (3):
> net: netfilter: Add kfuncs to allocate and insert CT
> net: netfilter: Add kfuncs to set and change CT status
> selftests/bpf: Add tests for new nf_conntrack kfuncs
>
> Documentation/bpf/index.rst | 1 +
> Documentation/bpf/kfuncs.rst | 170 ++++++++
> include/linux/bpf.h | 3 +-
> include/linux/btf.h | 65 ++--
> include/linux/btf_ids.h | 68 +++-
> include/net/netfilter/nf_conntrack_core.h | 19 +
> kernel/bpf/btf.c | 123 +++---
> kernel/bpf/verifier.c | 14 +-
> net/bpf/test_run.c | 75 ++--
> net/ipv4/bpf_tcp_ca.c | 18 +-
> net/ipv4/tcp_bbr.c | 24 +-
> net/ipv4/tcp_cubic.c | 20 +-
> net/ipv4/tcp_dctcp.c | 20 +-
> net/netfilter/nf_conntrack_bpf.c | 365 +++++++++++++-----
> net/netfilter/nf_conntrack_core.c | 62 +++
> net/netfilter/nf_conntrack_netlink.c | 54 +--
> tools/bpf/resolve_btfids/main.c | 40 +-
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +-
> .../testing/selftests/bpf/prog_tests/bpf_nf.c | 64 ++-
> .../testing/selftests/bpf/progs/test_bpf_nf.c | 85 +++-
> .../selftests/bpf/progs/test_bpf_nf_fail.c | 134 +++++++
> .../selftests/bpf/verifier/bpf_loop_inline.c | 1 +
> tools/testing/selftests/bpf/verifier/calls.c | 53 +++
> 23 files changed, 1139 insertions(+), 349 deletions(-)
> create mode 100644 Documentation/bpf/kfuncs.rst
> create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status
2022-07-21 17:28 ` [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Zvi Effron
@ 2022-07-21 18:01 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-21 18:01 UTC (permalink / raw)
To: Zvi Effron
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
On Thu, 21 Jul 2022 at 19:29, Zvi Effron <zeffron@riotgames.com> wrote:
>
> On Thu, Jul 21, 2022 at 6:43 AM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > Introduce the following new kfuncs:
> > - bpf_{xdp,skb}_ct_alloc
> > - bpf_ct_insert_entry
> > - bpf_ct_{set,change}_timeout
> > - bpf_ct_{set,change}_status
> >
> > The setting of timeout and status on allocated or inserted/looked up CT
> > is same as the ctnetlink interface, hence code is refactored and shared
> > with the kfuncs. It is ensured allocated CT cannot be passed to kfuncs
> > that expected inserted CT, and vice versa. Please see individual patches
> > for details.
> >
>
> Is it expected that using these helpers and the kernel's conntrack to manage
> connection state from XDP will outperform using maps and eBPF timers (for XDP
> use cases that don't have a userspace component that also needs the information
> in conntrack)? Have you done any benchmarking on the performance of using
> conntrack from XDP?
>
No, I haven't done any benchmarking against a BPF based conntrack.
The goal here is to give XDP and TC programs access to kernel's
existing conntrack, so that in cases where implementing one in BPF is
not desired/needed, the user can leverage the existing implementation
in the kernel.
> Thanks!
> --Zvi
>
> > Changelog:
> > ----------
> > v6 -> v7:
> > v6: https://lore.kernel.org/bpf/20220719132430.19993-1-memxor@gmail.com
> >
> > * Use .long to encode flags (Alexei)
> > * Fix description of KF_RET_NULL in documentation (Toke)
> >
> > v5 -> v6:
> > v5: https://lore.kernel.org/bpf/20220623192637.3866852-1-memxor@gmail.com
> >
> > * Introduce kfunc flags, rework verifier to work with them
> > * Add documentation for kfuncs
> > * Add comment explaining TRUSTED_ARGS kfunc flag (Alexei)
> > * Fix missing offset check for trusted arguments (Alexei)
> > * Change nf_conntrack test minimum delta value to 8
> >
> > v4 -> v5:
> > v4: https://lore.kernel.org/bpf/cover.1653600577.git.lorenzo@kernel.org
> >
> > * Drop read-only PTR_TO_BTF_ID approach, use struct nf_conn___init (Alexei)
> > * Drop acquire release pair code that is no longer required (Alexei)
> > * Disable writes into nf_conn, use dedicated helpers (Florian, Alexei)
> > * Refactor and share ctnetlink code for setting timeout and status
> > * Do strict type matching on finding __ref suffix on argument to
> > prevent passing nf_conn___init as nf_conn (offset = 0, match on walk)
> > * Remove bpf_ct_opts parameter from bpf_ct_insert_entry
> > * Update selftests for new additions, add more negative tests
> >
> > v3 -> v4:
> > v3: https://lore.kernel.org/bpf/cover.1652870182.git.lorenzo@kernel.org
> >
> > * split bpf_xdp_ct_add in bpf_xdp_ct_alloc/bpf_skb_ct_alloc and
> > bpf_ct_insert_entry
> > * add verifier code to properly populate/configure ct entry
> > * improve selftests
> >
> > v2 -> v3:
> > v2: https://lore.kernel.org/bpf/cover.1652372970.git.lorenzo@kernel.org
> >
> > * add bpf_xdp_ct_add and bpf_ct_refresh_timeout kfunc helpers
> > * remove conntrack dependency from selftests
> > * add support for forcing kfunc args to be referenced and related selftests
> >
> > v1 -> v2:
> > v1: https://lore.kernel.org/bpf/1327f8f5696ff2bc60400e8f3b79047914ccc837.1651595019.git.lorenzo@kernel.org
> >
> > * add bpf_ct_refresh_timeout kfunc selftest
> >
> > Kumar Kartikeya Dwivedi (10):
> > bpf: Introduce 8-byte BTF set
> > tools/resolve_btfids: Add support for 8-byte BTF sets
> > bpf: Switch to new kfunc flags infrastructure
> > bpf: Add support for forcing kfunc args to be trusted
> > bpf: Add documentation for kfuncs
> > net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup
> > net: netfilter: Add kfuncs to set and change CT timeout
> > selftests/bpf: Add verifier tests for trusted kfunc args
> > selftests/bpf: Add negative tests for new nf_conntrack kfuncs
> > selftests/bpf: Fix test_verifier failed test in unprivileged mode
> >
> > Lorenzo Bianconi (3):
> > net: netfilter: Add kfuncs to allocate and insert CT
> > net: netfilter: Add kfuncs to set and change CT status
> > selftests/bpf: Add tests for new nf_conntrack kfuncs
> >
> > Documentation/bpf/index.rst | 1 +
> > Documentation/bpf/kfuncs.rst | 170 ++++++++
> > include/linux/bpf.h | 3 +-
> > include/linux/btf.h | 65 ++--
> > include/linux/btf_ids.h | 68 +++-
> > include/net/netfilter/nf_conntrack_core.h | 19 +
> > kernel/bpf/btf.c | 123 +++---
> > kernel/bpf/verifier.c | 14 +-
> > net/bpf/test_run.c | 75 ++--
> > net/ipv4/bpf_tcp_ca.c | 18 +-
> > net/ipv4/tcp_bbr.c | 24 +-
> > net/ipv4/tcp_cubic.c | 20 +-
> > net/ipv4/tcp_dctcp.c | 20 +-
> > net/netfilter/nf_conntrack_bpf.c | 365 +++++++++++++-----
> > net/netfilter/nf_conntrack_core.c | 62 +++
> > net/netfilter/nf_conntrack_netlink.c | 54 +--
> > tools/bpf/resolve_btfids/main.c | 40 +-
> > .../selftests/bpf/bpf_testmod/bpf_testmod.c | 10 +-
> > .../testing/selftests/bpf/prog_tests/bpf_nf.c | 64 ++-
> > .../testing/selftests/bpf/progs/test_bpf_nf.c | 85 +++-
> > .../selftests/bpf/progs/test_bpf_nf_fail.c | 134 +++++++
> > .../selftests/bpf/verifier/bpf_loop_inline.c | 1 +
> > tools/testing/selftests/bpf/verifier/calls.c | 53 +++
> > 23 files changed, 1139 insertions(+), 349 deletions(-)
> > create mode 100644 Documentation/bpf/kfuncs.rst
> > create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 02/13] tools/resolve_btfids: Add support for 8-byte BTF sets
2022-07-21 13:42 ` [PATCH bpf-next v7 02/13] tools/resolve_btfids: Add support for 8-byte BTF sets Kumar Kartikeya Dwivedi
@ 2022-07-21 20:51 ` Jiri Olsa
0 siblings, 0 replies; 29+ messages in thread
From: Jiri Olsa @ 2022-07-21 20:51 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
On Thu, Jul 21, 2022 at 03:42:34PM +0200, Kumar Kartikeya Dwivedi wrote:
> A flag is a 4-byte symbol that may follow a BTF ID in a set8. This is
> used in the kernel to tag kfuncs in BTF sets with certain flags. Add
> support to adjust the sorting code so that it passes size as 8 bytes
> for 8-byte BTF sets.
>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
jirka
> ---
> tools/bpf/resolve_btfids/main.c | 40 ++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index 5d26f3c6f918..80cd7843c677 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -45,6 +45,19 @@
> * .zero 4
> * __BTF_ID__func__vfs_fallocate__4:
> * .zero 4
> + *
> + * set8 - store symbol size into first 4 bytes and sort following
> + * ID list
> + *
> + * __BTF_ID__set8__list:
> + * .zero 8
> + * list:
> + * __BTF_ID__func__vfs_getattr__3:
> + * .zero 4
> + * .word (1 << 0) | (1 << 2)
> + * __BTF_ID__func__vfs_fallocate__5:
> + * .zero 4
> + * .word (1 << 3) | (1 << 1) | (1 << 2)
> */
>
> #define _GNU_SOURCE
> @@ -72,6 +85,7 @@
> #define BTF_TYPEDEF "typedef"
> #define BTF_FUNC "func"
> #define BTF_SET "set"
> +#define BTF_SET8 "set8"
>
> #define ADDR_CNT 100
>
> @@ -84,6 +98,7 @@ struct btf_id {
> };
> int addr_cnt;
> bool is_set;
> + bool is_set8;
> Elf64_Addr addr[ADDR_CNT];
> };
>
> @@ -231,14 +246,14 @@ static char *get_id(const char *prefix_end)
> return id;
> }
>
> -static struct btf_id *add_set(struct object *obj, char *name)
> +static struct btf_id *add_set(struct object *obj, char *name, bool is_set8)
> {
> /*
> * __BTF_ID__set__name
> * name = ^
> * id = ^
> */
> - char *id = name + sizeof(BTF_SET "__") - 1;
> + char *id = name + (is_set8 ? sizeof(BTF_SET8 "__") : sizeof(BTF_SET "__")) - 1;
> int len = strlen(name);
>
> if (id >= name + len) {
> @@ -444,9 +459,21 @@ static int symbols_collect(struct object *obj)
> } else if (!strncmp(prefix, BTF_FUNC, sizeof(BTF_FUNC) - 1)) {
> obj->nr_funcs++;
> id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1);
> + /* set8 */
> + } else if (!strncmp(prefix, BTF_SET8, sizeof(BTF_SET8) - 1)) {
> + id = add_set(obj, prefix, true);
> + /*
> + * SET8 objects store list's count, which is encoded
> + * in symbol's size, together with 'cnt' field hence
> + * that - 1.
> + */
> + if (id) {
> + id->cnt = sym.st_size / sizeof(uint64_t) - 1;
> + id->is_set8 = true;
> + }
> /* set */
> } else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) {
> - id = add_set(obj, prefix);
> + id = add_set(obj, prefix, false);
> /*
> * SET objects store list's count, which is encoded
> * in symbol's size, together with 'cnt' field hence
> @@ -571,7 +598,8 @@ static int id_patch(struct object *obj, struct btf_id *id)
> int *ptr = data->d_buf;
> int i;
>
> - if (!id->id && !id->is_set)
> + /* For set, set8, id->id may be 0 */
> + if (!id->id && !id->is_set && !id->is_set8)
> pr_err("WARN: resolve_btfids: unresolved symbol %s\n", id->name);
>
> for (i = 0; i < id->addr_cnt; i++) {
> @@ -643,13 +671,13 @@ static int sets_patch(struct object *obj)
> }
>
> idx = idx / sizeof(int);
> - base = &ptr[idx] + 1;
> + base = &ptr[idx] + (id->is_set8 ? 2 : 1);
> cnt = ptr[idx];
>
> pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
> (idx + 1) * sizeof(int), cnt, id->name);
>
> - qsort(base, cnt, sizeof(int), cmp_id);
> + qsort(base, cnt, id->is_set8 ? sizeof(uint64_t) : sizeof(int), cmp_id);
>
> next = rb_next(next);
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
` (13 preceding siblings ...)
2022-07-21 17:28 ` [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Zvi Effron
@ 2022-07-22 4:10 ` patchwork-bot+netdevbpf
14 siblings, 0 replies; 29+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-07-22 4:10 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, ast, andrii, daniel, pablo, fw, brouer, toke, lorenzo,
netdev, netfilter-devel
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Thu, 21 Jul 2022 15:42:32 +0200 you wrote:
> Introduce the following new kfuncs:
> - bpf_{xdp,skb}_ct_alloc
> - bpf_ct_insert_entry
> - bpf_ct_{set,change}_timeout
> - bpf_ct_{set,change}_status
>
> The setting of timeout and status on allocated or inserted/looked up CT
> is same as the ctnetlink interface, hence code is refactored and shared
> with the kfuncs. It is ensured allocated CT cannot be passed to kfuncs
> that expected inserted CT, and vice versa. Please see individual patches
> for details.
>
> [...]
Here is the summary with links:
- [bpf-next,v7,01/13] bpf: Introduce 8-byte BTF set
https://git.kernel.org/bpf/bpf-next/c/ab21d6063c01
- [bpf-next,v7,02/13] tools/resolve_btfids: Add support for 8-byte BTF sets
https://git.kernel.org/bpf/bpf-next/c/ef2c6f370a63
- [bpf-next,v7,03/13] bpf: Switch to new kfunc flags infrastructure
https://git.kernel.org/bpf/bpf-next/c/a4703e318432
- [bpf-next,v7,04/13] bpf: Add support for forcing kfunc args to be trusted
https://git.kernel.org/bpf/bpf-next/c/56e948ffc098
- [bpf-next,v7,05/13] bpf: Add documentation for kfuncs
https://git.kernel.org/bpf/bpf-next/c/63e564ebd1fd
- [bpf-next,v7,06/13] net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup
https://git.kernel.org/bpf/bpf-next/c/aed8ee7feb44
- [bpf-next,v7,07/13] net: netfilter: Add kfuncs to allocate and insert CT
https://git.kernel.org/bpf/bpf-next/c/d7e79c97c00c
- [bpf-next,v7,08/13] net: netfilter: Add kfuncs to set and change CT timeout
https://git.kernel.org/bpf/bpf-next/c/0b3892364431
- [bpf-next,v7,09/13] net: netfilter: Add kfuncs to set and change CT status
https://git.kernel.org/bpf/bpf-next/c/ef69aa3a986e
- [bpf-next,v7,10/13] selftests/bpf: Add verifier tests for trusted kfunc args
https://git.kernel.org/bpf/bpf-next/c/8dd5e75683f7
- [bpf-next,v7,11/13] selftests/bpf: Add tests for new nf_conntrack kfuncs
https://git.kernel.org/bpf/bpf-next/c/6eb7fba007a7
- [bpf-next,v7,12/13] selftests/bpf: Add negative tests for new nf_conntrack kfuncs
https://git.kernel.org/bpf/bpf-next/c/c6f420ac9d25
- [bpf-next,v7,13/13] selftests/bpf: Fix test_verifier failed test in unprivileged mode
https://git.kernel.org/bpf/bpf-next/c/e3fa4735f04d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-21 13:42 ` [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted Kumar Kartikeya Dwivedi
@ 2022-07-22 4:10 ` Alexei Starovoitov
2022-07-22 10:26 ` Kumar Kartikeya Dwivedi
2022-07-25 9:52 ` Roberto Sassu
1 sibling, 1 reply; 29+ messages in thread
From: Alexei Starovoitov @ 2022-07-22 4:10 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
On Thu, Jul 21, 2022 at 03:42:36PM +0200, Kumar Kartikeya Dwivedi wrote:
> +/* Trusted arguments are those which are meant to be referenced arguments with
> + * unchanged offset. It is used to enforce that pointers obtained from acquire
> + * kfuncs remain unmodified when being passed to helpers taking trusted args.
> + *
> + * Consider
> + * struct foo {
> + * int data;
> + * struct foo *next;
> + * };
> + *
> + * struct bar {
> + * int data;
> + * struct foo f;
> + * };
> + *
> + * struct foo *f = alloc_foo(); // Acquire kfunc
> + * struct bar *b = alloc_bar(); // Acquire kfunc
> + *
> + * If a kfunc set_foo_data() wants to operate only on the allocated object, it
> + * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage like:
> + *
> + * set_foo_data(f, 42); // Allowed
> + * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
> + * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
> + * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
I think you meant to swap above two comments ?
That's what I did while applying.
Also fixed typo in Fixes tag in patch 13. It was missing a letter in sha.
Since there are 3 other pending patchsets in patchwork that add new kfuncs
this cleanup of kfunc registration couldn't have come at better time.
Thank you for doing this work.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT
2022-07-21 13:42 ` [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT Kumar Kartikeya Dwivedi
@ 2022-07-22 9:02 ` Pablo Neira Ayuso
2022-07-22 9:39 ` Kumar Kartikeya Dwivedi
0 siblings, 1 reply; 29+ messages in thread
From: Pablo Neira Ayuso @ 2022-07-22 9:02 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, netdev, netfilter-devel
Hi,
On Thu, Jul 21, 2022 at 03:42:39PM +0200, Kumar Kartikeya Dwivedi wrote:
> diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
> index 37866c8386e2..83a60c684e6c 100644
> --- a/include/net/netfilter/nf_conntrack_core.h
> +++ b/include/net/netfilter/nf_conntrack_core.h
> @@ -84,4 +84,19 @@ void nf_conntrack_lock(spinlock_t *lock);
>
> extern spinlock_t nf_conntrack_expect_lock;
>
> +/* ctnetlink code shared by both ctnetlink and nf_conntrack_bpf */
> +
> +#if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> + (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) || \
> + IS_ENABLED(CONFIG_NF_CT_NETLINK))
There must be a better way to do this without ifdef pollution?
Could you fix this?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT
2022-07-22 9:02 ` Pablo Neira Ayuso
@ 2022-07-22 9:39 ` Kumar Kartikeya Dwivedi
2022-07-23 7:50 ` Pablo Neira Ayuso
0 siblings, 1 reply; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-22 9:39 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: bpf, Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, netdev, netfilter-devel
On Fri, 22 Jul 2022 at 11:02, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi,
>
> On Thu, Jul 21, 2022 at 03:42:39PM +0200, Kumar Kartikeya Dwivedi wrote:
> > diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
> > index 37866c8386e2..83a60c684e6c 100644
> > --- a/include/net/netfilter/nf_conntrack_core.h
> > +++ b/include/net/netfilter/nf_conntrack_core.h
> > @@ -84,4 +84,19 @@ void nf_conntrack_lock(spinlock_t *lock);
> >
> > extern spinlock_t nf_conntrack_expect_lock;
> >
> > +/* ctnetlink code shared by both ctnetlink and nf_conntrack_bpf */
> > +
> > +#if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > + (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) || \
> > + IS_ENABLED(CONFIG_NF_CT_NETLINK))
>
> There must be a better way to do this without ifdef pollution?
>
> Could you fix this?
I can just remove the ifdefs completely. The first part of the ifdef
is the correct way to detect BPF support for nf_conntrack, the second
is for ct netlink. These are the only two users. But it's not a lot of
code, so until it grows too much we can compile it unconditionally.
Or do you have anything else in mind (like defining a macro for the
bpf one and making the ifdef look less ugly)?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-22 4:10 ` Alexei Starovoitov
@ 2022-07-22 10:26 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-22 10:26 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
On Fri, 22 Jul 2022 at 06:11, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Jul 21, 2022 at 03:42:36PM +0200, Kumar Kartikeya Dwivedi wrote:
> > +/* Trusted arguments are those which are meant to be referenced arguments with
> > + * unchanged offset. It is used to enforce that pointers obtained from acquire
> > + * kfuncs remain unmodified when being passed to helpers taking trusted args.
> > + *
> > + * Consider
> > + * struct foo {
> > + * int data;
> > + * struct foo *next;
> > + * };
> > + *
> > + * struct bar {
> > + * int data;
> > + * struct foo f;
> > + * };
> > + *
> > + * struct foo *f = alloc_foo(); // Acquire kfunc
> > + * struct bar *b = alloc_bar(); // Acquire kfunc
> > + *
> > + * If a kfunc set_foo_data() wants to operate only on the allocated object, it
> > + * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage like:
> > + *
> > + * set_foo_data(f, 42); // Allowed
> > + * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
> > + * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
> > + * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
>
> I think you meant to swap above two comments ?
> That's what I did while applying.
>
> Also fixed typo in Fixes tag in patch 13. It was missing a letter in sha.
>
> Since there are 3 other pending patchsets in patchwork that add new kfuncs
> this cleanup of kfunc registration couldn't have come at better time.
>
> Thank you for doing this work.
Thank you for doing the fixups!
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT
2022-07-22 9:39 ` Kumar Kartikeya Dwivedi
@ 2022-07-23 7:50 ` Pablo Neira Ayuso
2022-07-25 8:52 ` Kumar Kartikeya Dwivedi
0 siblings, 1 reply; 29+ messages in thread
From: Pablo Neira Ayuso @ 2022-07-23 7:50 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, netdev, netfilter-devel
On Fri, Jul 22, 2022 at 11:39:49AM +0200, Kumar Kartikeya Dwivedi wrote:
> On Fri, 22 Jul 2022 at 11:02, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > Hi,
> >
> > On Thu, Jul 21, 2022 at 03:42:39PM +0200, Kumar Kartikeya Dwivedi wrote:
> > > diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
> > > index 37866c8386e2..83a60c684e6c 100644
> > > --- a/include/net/netfilter/nf_conntrack_core.h
> > > +++ b/include/net/netfilter/nf_conntrack_core.h
> > > @@ -84,4 +84,19 @@ void nf_conntrack_lock(spinlock_t *lock);
> > >
> > > extern spinlock_t nf_conntrack_expect_lock;
> > >
> > > +/* ctnetlink code shared by both ctnetlink and nf_conntrack_bpf */
> > > +
> > > +#if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > > + (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) || \
> > > + IS_ENABLED(CONFIG_NF_CT_NETLINK))
> >
> > There must be a better way to do this without ifdef pollution?
> >
> > Could you fix this?
>
> I can just remove the ifdefs completely. The first part of the ifdef
> is the correct way to detect BPF support for nf_conntrack, the second
> is for ct netlink. These are the only two users. But it's not a lot of
> code, so until it grows too much we can compile it unconditionally.
I would suggest to compile in these small functions unconditionally.
> Or do you have anything else in mind (like defining a macro for the
> bpf one and making the ifdef look less ugly)?
it's the ifdef pollution that it would be good to avoid IMO.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT
2022-07-23 7:50 ` Pablo Neira Ayuso
@ 2022-07-25 8:52 ` Kumar Kartikeya Dwivedi
0 siblings, 0 replies; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-25 8:52 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: bpf, Lorenzo Bianconi, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, netdev, netfilter-devel
On Sat, 23 Jul 2022 at 09:50, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Fri, Jul 22, 2022 at 11:39:49AM +0200, Kumar Kartikeya Dwivedi wrote:
> > On Fri, 22 Jul 2022 at 11:02, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Jul 21, 2022 at 03:42:39PM +0200, Kumar Kartikeya Dwivedi wrote:
> > > > diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
> > > > index 37866c8386e2..83a60c684e6c 100644
> > > > --- a/include/net/netfilter/nf_conntrack_core.h
> > > > +++ b/include/net/netfilter/nf_conntrack_core.h
> > > > @@ -84,4 +84,19 @@ void nf_conntrack_lock(spinlock_t *lock);
> > > >
> > > > extern spinlock_t nf_conntrack_expect_lock;
> > > >
> > > > +/* ctnetlink code shared by both ctnetlink and nf_conntrack_bpf */
> > > > +
> > > > +#if (IS_BUILTIN(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) || \
> > > > + (IS_MODULE(CONFIG_NF_CONNTRACK) && IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES) || \
> > > > + IS_ENABLED(CONFIG_NF_CT_NETLINK))
> > >
> > > There must be a better way to do this without ifdef pollution?
> > >
> > > Could you fix this?
> >
> > I can just remove the ifdefs completely. The first part of the ifdef
> > is the correct way to detect BPF support for nf_conntrack, the second
> > is for ct netlink. These are the only two users. But it's not a lot of
> > code, so until it grows too much we can compile it unconditionally.
>
> I would suggest to compile in these small functions unconditionally.
>
> > Or do you have anything else in mind (like defining a macro for the
> > bpf one and making the ifdef look less ugly)?
>
> it's the ifdef pollution that it would be good to avoid IMO.
Ok, I have sent a fix to remove these ifdefs. We can revisit this when
more shared functions appear.
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-21 13:42 ` [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted Kumar Kartikeya Dwivedi
2022-07-22 4:10 ` Alexei Starovoitov
@ 2022-07-25 9:52 ` Roberto Sassu
2022-07-26 9:30 ` Kumar Kartikeya Dwivedi
1 sibling, 1 reply; 29+ messages in thread
From: Roberto Sassu @ 2022-07-25 9:52 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
> From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> Sent: Thursday, July 21, 2022 3:43 PM
> Teach the verifier to detect a new KF_TRUSTED_ARGS kfunc flag, which
> means each pointer argument must be trusted, which we define as a
> pointer that is referenced (has non-zero ref_obj_id) and also needs to
> have its offset unchanged, similar to how release functions expect their
> argument. This allows a kfunc to receive pointer arguments unchanged
> from the result of the acquire kfunc.
>
> This is required to ensure that kfunc that operate on some object only
> work on acquired pointers and not normal PTR_TO_BTF_ID with same type
> which can be obtained by pointer walking. The restrictions applied to
> release arguments also apply to trusted arguments. This implies that
> strict type matching (not deducing type by recursively following members
> at offset) and OBJ_RELEASE offset checks (ensuring they are zero) are
> used for trusted pointer arguments.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
> include/linux/btf.h | 32 ++++++++++++++++++++++++++++++++
> kernel/bpf/btf.c | 17 ++++++++++++++---
> net/bpf/test_run.c | 5 +++++
> 3 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 6dfc6eaf7f8c..cb63aa71e82f 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -17,6 +17,38 @@
> #define KF_RELEASE (1 << 1) /* kfunc is a release function */
> #define KF_RET_NULL (1 << 2) /* kfunc returns a pointer that may be NULL */
> #define KF_KPTR_GET (1 << 3) /* kfunc returns reference to a kptr */
> +/* Trusted arguments are those which are meant to be referenced arguments
> with
> + * unchanged offset. It is used to enforce that pointers obtained from acquire
> + * kfuncs remain unmodified when being passed to helpers taking trusted args.
> + *
> + * Consider
> + * struct foo {
> + * int data;
> + * struct foo *next;
> + * };
> + *
> + * struct bar {
> + * int data;
> + * struct foo f;
> + * };
> + *
> + * struct foo *f = alloc_foo(); // Acquire kfunc
> + * struct bar *b = alloc_bar(); // Acquire kfunc
> + *
> + * If a kfunc set_foo_data() wants to operate only on the allocated object, it
> + * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage like:
> + *
> + * set_foo_data(f, 42); // Allowed
> + * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
> + * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
> + * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
> + *
> + * In the final case, usually for the purposes of type matching, it is deduced
> + * by looking at the type of the member at the offset, but due to the
> + * requirement of trusted argument, this deduction will be strict and not done
> + * for this case.
> + */
> +#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer
> arguments */
Hi Kumar
would it make sense to introduce per-parameter flags? I have a function
that has several parameters, but only one is referenced.
Thanks
Roberto
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-25 9:52 ` Roberto Sassu
@ 2022-07-26 9:30 ` Kumar Kartikeya Dwivedi
2022-07-26 10:02 ` Roberto Sassu
0 siblings, 1 reply; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-26 9:30 UTC (permalink / raw)
To: Roberto Sassu
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
On Mon, 25 Jul 2022 at 11:52, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> > From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> > Sent: Thursday, July 21, 2022 3:43 PM
> > Teach the verifier to detect a new KF_TRUSTED_ARGS kfunc flag, which
> > means each pointer argument must be trusted, which we define as a
> > pointer that is referenced (has non-zero ref_obj_id) and also needs to
> > have its offset unchanged, similar to how release functions expect their
> > argument. This allows a kfunc to receive pointer arguments unchanged
> > from the result of the acquire kfunc.
> >
> > This is required to ensure that kfunc that operate on some object only
> > work on acquired pointers and not normal PTR_TO_BTF_ID with same type
> > which can be obtained by pointer walking. The restrictions applied to
> > release arguments also apply to trusted arguments. This implies that
> > strict type matching (not deducing type by recursively following members
> > at offset) and OBJ_RELEASE offset checks (ensuring they are zero) are
> > used for trusted pointer arguments.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> > include/linux/btf.h | 32 ++++++++++++++++++++++++++++++++
> > kernel/bpf/btf.c | 17 ++++++++++++++---
> > net/bpf/test_run.c | 5 +++++
> > 3 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > index 6dfc6eaf7f8c..cb63aa71e82f 100644
> > --- a/include/linux/btf.h
> > +++ b/include/linux/btf.h
> > @@ -17,6 +17,38 @@
> > #define KF_RELEASE (1 << 1) /* kfunc is a release function */
> > #define KF_RET_NULL (1 << 2) /* kfunc returns a pointer that may be NULL */
> > #define KF_KPTR_GET (1 << 3) /* kfunc returns reference to a kptr */
> > +/* Trusted arguments are those which are meant to be referenced arguments
> > with
> > + * unchanged offset. It is used to enforce that pointers obtained from acquire
> > + * kfuncs remain unmodified when being passed to helpers taking trusted args.
> > + *
> > + * Consider
> > + * struct foo {
> > + * int data;
> > + * struct foo *next;
> > + * };
> > + *
> > + * struct bar {
> > + * int data;
> > + * struct foo f;
> > + * };
> > + *
> > + * struct foo *f = alloc_foo(); // Acquire kfunc
> > + * struct bar *b = alloc_bar(); // Acquire kfunc
> > + *
> > + * If a kfunc set_foo_data() wants to operate only on the allocated object, it
> > + * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage like:
> > + *
> > + * set_foo_data(f, 42); // Allowed
> > + * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
> > + * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
> > + * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
> > + *
> > + * In the final case, usually for the purposes of type matching, it is deduced
> > + * by looking at the type of the member at the offset, but due to the
> > + * requirement of trusted argument, this deduction will be strict and not done
> > + * for this case.
> > + */
> > +#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer
> > arguments */
>
> Hi Kumar
>
> would it make sense to introduce per-parameter flags? I have a function
> that has several parameters, but only one is referenced.
>
I have a patch for that in my local branch, I can fix it up and post
it. But first, can you give an example of where you think you need it?
> Thanks
>
> Roberto
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-26 9:30 ` Kumar Kartikeya Dwivedi
@ 2022-07-26 10:02 ` Roberto Sassu
2022-07-26 12:55 ` Kumar Kartikeya Dwivedi
0 siblings, 1 reply; 29+ messages in thread
From: Roberto Sassu @ 2022-07-26 10:02 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
> From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> Sent: Tuesday, July 26, 2022 11:30 AM
> On Mon, 25 Jul 2022 at 11:52, Roberto Sassu <roberto.sassu@huawei.com>
> wrote:
> >
> > > From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> > > Sent: Thursday, July 21, 2022 3:43 PM
> > > Teach the verifier to detect a new KF_TRUSTED_ARGS kfunc flag, which
> > > means each pointer argument must be trusted, which we define as a
> > > pointer that is referenced (has non-zero ref_obj_id) and also needs to
> > > have its offset unchanged, similar to how release functions expect their
> > > argument. This allows a kfunc to receive pointer arguments unchanged
> > > from the result of the acquire kfunc.
> > >
> > > This is required to ensure that kfunc that operate on some object only
> > > work on acquired pointers and not normal PTR_TO_BTF_ID with same type
> > > which can be obtained by pointer walking. The restrictions applied to
> > > release arguments also apply to trusted arguments. This implies that
> > > strict type matching (not deducing type by recursively following members
> > > at offset) and OBJ_RELEASE offset checks (ensuring they are zero) are
> > > used for trusted pointer arguments.
> > >
> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > ---
> > > include/linux/btf.h | 32 ++++++++++++++++++++++++++++++++
> > > kernel/bpf/btf.c | 17 ++++++++++++++---
> > > net/bpf/test_run.c | 5 +++++
> > > 3 files changed, 51 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > > index 6dfc6eaf7f8c..cb63aa71e82f 100644
> > > --- a/include/linux/btf.h
> > > +++ b/include/linux/btf.h
> > > @@ -17,6 +17,38 @@
> > > #define KF_RELEASE (1 << 1) /* kfunc is a release function */
> > > #define KF_RET_NULL (1 << 2) /* kfunc returns a pointer that may be NULL
> */
> > > #define KF_KPTR_GET (1 << 3) /* kfunc returns reference to a kptr */
> > > +/* Trusted arguments are those which are meant to be referenced
> arguments
> > > with
> > > + * unchanged offset. It is used to enforce that pointers obtained from
> acquire
> > > + * kfuncs remain unmodified when being passed to helpers taking trusted
> args.
> > > + *
> > > + * Consider
> > > + * struct foo {
> > > + * int data;
> > > + * struct foo *next;
> > > + * };
> > > + *
> > > + * struct bar {
> > > + * int data;
> > > + * struct foo f;
> > > + * };
> > > + *
> > > + * struct foo *f = alloc_foo(); // Acquire kfunc
> > > + * struct bar *b = alloc_bar(); // Acquire kfunc
> > > + *
> > > + * If a kfunc set_foo_data() wants to operate only on the allocated object,
> it
> > > + * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage like:
> > > + *
> > > + * set_foo_data(f, 42); // Allowed
> > > + * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
> > > + * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
> > > + * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
> > > + *
> > > + * In the final case, usually for the purposes of type matching, it is deduced
> > > + * by looking at the type of the member at the offset, but due to the
> > > + * requirement of trusted argument, this deduction will be strict and not
> done
> > > + * for this case.
> > > + */
> > > +#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer
> > > arguments */
> >
> > Hi Kumar
> >
> > would it make sense to introduce per-parameter flags? I have a function
> > that has several parameters, but only one is referenced.
> >
>
> I have a patch for that in my local branch, I can fix it up and post
> it. But first, can you give an example of where you think you need it?
I have pushed the complete patch set here, for testing:
https://github.com/robertosassu/vmtest/tree/bpf-verify-sig-v9/travis-ci/diffs
I rebased to bpf-next/master, and introduced KF_SLEEPABLE (similar
functionality of " btf: Add a new kfunc set which allows to mark
a function to be sleepable" from Benjamin Tissoires).
The patch where I would use per-parameter KF_TRUSTED_ARGS is
number 8. I also used your new API in patch 7 and it works well.
I didn't repost, as I'm waiting for comments on v8.
Thanks
Roberto
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-26 10:02 ` Roberto Sassu
@ 2022-07-26 12:55 ` Kumar Kartikeya Dwivedi
2022-07-26 12:58 ` Roberto Sassu
0 siblings, 1 reply; 29+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2022-07-26 12:55 UTC (permalink / raw)
To: Roberto Sassu
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
On Tue, 26 Jul 2022 at 12:02, Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> > From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> > Sent: Tuesday, July 26, 2022 11:30 AM
> > On Mon, 25 Jul 2022 at 11:52, Roberto Sassu <roberto.sassu@huawei.com>
> > wrote:
> > >
> > > > From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> > > > Sent: Thursday, July 21, 2022 3:43 PM
> > > > Teach the verifier to detect a new KF_TRUSTED_ARGS kfunc flag, which
> > > > means each pointer argument must be trusted, which we define as a
> > > > pointer that is referenced (has non-zero ref_obj_id) and also needs to
> > > > have its offset unchanged, similar to how release functions expect their
> > > > argument. This allows a kfunc to receive pointer arguments unchanged
> > > > from the result of the acquire kfunc.
> > > >
> > > > This is required to ensure that kfunc that operate on some object only
> > > > work on acquired pointers and not normal PTR_TO_BTF_ID with same type
> > > > which can be obtained by pointer walking. The restrictions applied to
> > > > release arguments also apply to trusted arguments. This implies that
> > > > strict type matching (not deducing type by recursively following members
> > > > at offset) and OBJ_RELEASE offset checks (ensuring they are zero) are
> > > > used for trusted pointer arguments.
> > > >
> > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > > ---
> > > > include/linux/btf.h | 32 ++++++++++++++++++++++++++++++++
> > > > kernel/bpf/btf.c | 17 ++++++++++++++---
> > > > net/bpf/test_run.c | 5 +++++
> > > > 3 files changed, 51 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > > > index 6dfc6eaf7f8c..cb63aa71e82f 100644
> > > > --- a/include/linux/btf.h
> > > > +++ b/include/linux/btf.h
> > > > @@ -17,6 +17,38 @@
> > > > #define KF_RELEASE (1 << 1) /* kfunc is a release function */
> > > > #define KF_RET_NULL (1 << 2) /* kfunc returns a pointer that may be NULL
> > */
> > > > #define KF_KPTR_GET (1 << 3) /* kfunc returns reference to a kptr */
> > > > +/* Trusted arguments are those which are meant to be referenced
> > arguments
> > > > with
> > > > + * unchanged offset. It is used to enforce that pointers obtained from
> > acquire
> > > > + * kfuncs remain unmodified when being passed to helpers taking trusted
> > args.
> > > > + *
> > > > + * Consider
> > > > + * struct foo {
> > > > + * int data;
> > > > + * struct foo *next;
> > > > + * };
> > > > + *
> > > > + * struct bar {
> > > > + * int data;
> > > > + * struct foo f;
> > > > + * };
> > > > + *
> > > > + * struct foo *f = alloc_foo(); // Acquire kfunc
> > > > + * struct bar *b = alloc_bar(); // Acquire kfunc
> > > > + *
> > > > + * If a kfunc set_foo_data() wants to operate only on the allocated object,
> > it
> > > > + * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage like:
> > > > + *
> > > > + * set_foo_data(f, 42); // Allowed
> > > > + * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
> > > > + * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
> > > > + * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
> > > > + *
> > > > + * In the final case, usually for the purposes of type matching, it is deduced
> > > > + * by looking at the type of the member at the offset, but due to the
> > > > + * requirement of trusted argument, this deduction will be strict and not
> > done
> > > > + * for this case.
> > > > + */
> > > > +#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer
> > > > arguments */
> > >
> > > Hi Kumar
> > >
> > > would it make sense to introduce per-parameter flags? I have a function
> > > that has several parameters, but only one is referenced.
> > >
> >
> > I have a patch for that in my local branch, I can fix it up and post
> > it. But first, can you give an example of where you think you need it?
>
> I have pushed the complete patch set here, for testing:
>
> https://github.com/robertosassu/vmtest/tree/bpf-verify-sig-v9/travis-ci/diffs
>
> I rebased to bpf-next/master, and introduced KF_SLEEPABLE (similar
> functionality of " btf: Add a new kfunc set which allows to mark
> a function to be sleepable" from Benjamin Tissoires).
>
> The patch where I would use per-parameter KF_TRUSTED_ARGS is
> number 8. I also used your new API in patch 7 and it works well.
>
Ok, looks like you'll need it for the struct key * argument as there
are multiple pointers in the argument list and not all of them need to
be trusted. I will clean up and post the patch with a test later today
to the list.
> I didn't repost, as I'm waiting for comments on v8.
>
> Thanks
>
> Roberto
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted
2022-07-26 12:55 ` Kumar Kartikeya Dwivedi
@ 2022-07-26 12:58 ` Roberto Sassu
0 siblings, 0 replies; 29+ messages in thread
From: Roberto Sassu @ 2022-07-26 12:58 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Pablo Neira Ayuso, Florian Westphal, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen, Lorenzo Bianconi, netdev,
netfilter-devel
> From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> Sent: Tuesday, July 26, 2022 2:56 PM
> On Tue, 26 Jul 2022 at 12:02, Roberto Sassu <roberto.sassu@huawei.com>
> wrote:
> >
> > > From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> > > Sent: Tuesday, July 26, 2022 11:30 AM
> > > On Mon, 25 Jul 2022 at 11:52, Roberto Sassu <roberto.sassu@huawei.com>
> > > wrote:
> > > >
> > > > > From: Kumar Kartikeya Dwivedi [mailto:memxor@gmail.com]
> > > > > Sent: Thursday, July 21, 2022 3:43 PM
> > > > > Teach the verifier to detect a new KF_TRUSTED_ARGS kfunc flag, which
> > > > > means each pointer argument must be trusted, which we define as a
> > > > > pointer that is referenced (has non-zero ref_obj_id) and also needs to
> > > > > have its offset unchanged, similar to how release functions expect their
> > > > > argument. This allows a kfunc to receive pointer arguments unchanged
> > > > > from the result of the acquire kfunc.
> > > > >
> > > > > This is required to ensure that kfunc that operate on some object only
> > > > > work on acquired pointers and not normal PTR_TO_BTF_ID with same
> type
> > > > > which can be obtained by pointer walking. The restrictions applied to
> > > > > release arguments also apply to trusted arguments. This implies that
> > > > > strict type matching (not deducing type by recursively following members
> > > > > at offset) and OBJ_RELEASE offset checks (ensuring they are zero) are
> > > > > used for trusted pointer arguments.
> > > > >
> > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > > > ---
> > > > > include/linux/btf.h | 32 ++++++++++++++++++++++++++++++++
> > > > > kernel/bpf/btf.c | 17 ++++++++++++++---
> > > > > net/bpf/test_run.c | 5 +++++
> > > > > 3 files changed, 51 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > > > > index 6dfc6eaf7f8c..cb63aa71e82f 100644
> > > > > --- a/include/linux/btf.h
> > > > > +++ b/include/linux/btf.h
> > > > > @@ -17,6 +17,38 @@
> > > > > #define KF_RELEASE (1 << 1) /* kfunc is a release function */
> > > > > #define KF_RET_NULL (1 << 2) /* kfunc returns a pointer that may be
> NULL
> > > */
> > > > > #define KF_KPTR_GET (1 << 3) /* kfunc returns reference to a kptr */
> > > > > +/* Trusted arguments are those which are meant to be referenced
> > > arguments
> > > > > with
> > > > > + * unchanged offset. It is used to enforce that pointers obtained from
> > > acquire
> > > > > + * kfuncs remain unmodified when being passed to helpers taking
> trusted
> > > args.
> > > > > + *
> > > > > + * Consider
> > > > > + * struct foo {
> > > > > + * int data;
> > > > > + * struct foo *next;
> > > > > + * };
> > > > > + *
> > > > > + * struct bar {
> > > > > + * int data;
> > > > > + * struct foo f;
> > > > > + * };
> > > > > + *
> > > > > + * struct foo *f = alloc_foo(); // Acquire kfunc
> > > > > + * struct bar *b = alloc_bar(); // Acquire kfunc
> > > > > + *
> > > > > + * If a kfunc set_foo_data() wants to operate only on the allocated
> object,
> > > it
> > > > > + * will set the KF_TRUSTED_ARGS flag, which will prevent unsafe usage
> like:
> > > > > + *
> > > > > + * set_foo_data(f, 42); // Allowed
> > > > > + * set_foo_data(f->next, 42); // Rejected, non-referenced pointer
> > > > > + * set_foo_data(&f->next, 42);// Rejected, referenced, but bad offset
> > > > > + * set_foo_data(&b->f, 42); // Rejected, referenced, but wrong type
> > > > > + *
> > > > > + * In the final case, usually for the purposes of type matching, it is
> deduced
> > > > > + * by looking at the type of the member at the offset, but due to the
> > > > > + * requirement of trusted argument, this deduction will be strict and not
> > > done
> > > > > + * for this case.
> > > > > + */
> > > > > +#define KF_TRUSTED_ARGS (1 << 4) /* kfunc only takes trusted pointer
> > > > > arguments */
> > > >
> > > > Hi Kumar
> > > >
> > > > would it make sense to introduce per-parameter flags? I have a function
> > > > that has several parameters, but only one is referenced.
> > > >
> > >
> > > I have a patch for that in my local branch, I can fix it up and post
> > > it. But first, can you give an example of where you think you need it?
> >
> > I have pushed the complete patch set here, for testing:
> >
> > https://github.com/robertosassu/vmtest/tree/bpf-verify-sig-v9/travis-ci/diffs
> >
> > I rebased to bpf-next/master, and introduced KF_SLEEPABLE (similar
> > functionality of " btf: Add a new kfunc set which allows to mark
> > a function to be sleepable" from Benjamin Tissoires).
> >
> > The patch where I would use per-parameter KF_TRUSTED_ARGS is
> > number 8. I also used your new API in patch 7 and it works well.
> >
>
> Ok, looks like you'll need it for the struct key * argument as there
> are multiple pointers in the argument list and not all of them need to
> be trusted. I will clean up and post the patch with a test later today
> to the list.
Yes, thanks a lot!
Roberto
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2022-07-26 12:58 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21 13:42 [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 01/13] bpf: Introduce 8-byte BTF set Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 02/13] tools/resolve_btfids: Add support for 8-byte BTF sets Kumar Kartikeya Dwivedi
2022-07-21 20:51 ` Jiri Olsa
2022-07-21 13:42 ` [PATCH bpf-next v7 03/13] bpf: Switch to new kfunc flags infrastructure Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 04/13] bpf: Add support for forcing kfunc args to be trusted Kumar Kartikeya Dwivedi
2022-07-22 4:10 ` Alexei Starovoitov
2022-07-22 10:26 ` Kumar Kartikeya Dwivedi
2022-07-25 9:52 ` Roberto Sassu
2022-07-26 9:30 ` Kumar Kartikeya Dwivedi
2022-07-26 10:02 ` Roberto Sassu
2022-07-26 12:55 ` Kumar Kartikeya Dwivedi
2022-07-26 12:58 ` Roberto Sassu
2022-07-21 13:42 ` [PATCH bpf-next v7 05/13] bpf: Add documentation for kfuncs Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 06/13] net: netfilter: Deduplicate code in bpf_{xdp,skb}_ct_lookup Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 07/13] net: netfilter: Add kfuncs to allocate and insert CT Kumar Kartikeya Dwivedi
2022-07-22 9:02 ` Pablo Neira Ayuso
2022-07-22 9:39 ` Kumar Kartikeya Dwivedi
2022-07-23 7:50 ` Pablo Neira Ayuso
2022-07-25 8:52 ` Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 08/13] net: netfilter: Add kfuncs to set and change CT timeout Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 09/13] net: netfilter: Add kfuncs to set and change CT status Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 10/13] selftests/bpf: Add verifier tests for trusted kfunc args Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 11/13] selftests/bpf: Add tests for new nf_conntrack kfuncs Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 12/13] selftests/bpf: Add negative " Kumar Kartikeya Dwivedi
2022-07-21 13:42 ` [PATCH bpf-next v7 13/13] selftests/bpf: Fix test_verifier failed test in unprivileged mode Kumar Kartikeya Dwivedi
2022-07-21 17:28 ` [PATCH bpf-next v7 00/13] New nf_conntrack kfuncs for insertion, changing timeout, status Zvi Effron
2022-07-21 18:01 ` Kumar Kartikeya Dwivedi
2022-07-22 4:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).