[RFC PATCH 01/11] bpf: context casting for tail call

From: Kris Van Hees
Date: Tue May 21 2019 - 16:44:05 EST

Next message: Kris Van Hees: "[RFC PATCH 05/11] trace: update Kconfig and Makefile to include DTrace"
Previous message: Kees Cook: "Re: [PATCH v3] kernel: fix typos and some coding style in comments"
In reply to: Kris Van Hees: "[RFC PATCH 11/11] dtrace: make use of writable buffers in BPF"
Next in thread: Kris Van Hees: "[RFC PATCH 05/11] trace: update Kconfig and Makefile to include DTrace"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Currently BPF programs are executed with a context that is provided by
code that initiates the execution. Tracing tools that want to make use
of existing probes and events that allow BPF programs to be attached to
them are thus limited to the context information provided by the probe
or event source. Often, more context is needed to allow tracing tools
the ablity to implement more complex constructs (e.g. more state-full
tracing).

This patch extends the tail-call mechanism to allow a BPF program of
one type to call a BPF program of another type.

BPF program types can specify two new operations in struct bpf_prog_ops:
- bool is_valid_tail_call(enum bpf_prog_type stype)
This function is called from bpf_prog_array_valid_tail_call()
which is called from bpf_check_tail_call()
which is called from bpf_prog_select_runtime()
which is called from bpf_prog_load() right after the
verifier finishes processing the program. It is called for every
map of type BPF_MAP_TYPE_PROG_ARRAY, and is passed the type of the
program that is being loaded and therefore will be the origin of
tail calls. It returns true if tail calls from the source BPF
program type to the implementing program type are allowed.

- void *convert_ctx(enum bpf_prog_type stype, void *ctx)
This function is called during the execution of a BPF tail-call.
It returns a valid context for the implementing BPF program type,
based on the passed context pointer (ctx) for BPF program type
stype.

The program array holding BPF programs that you can tail-call into
continues to require that all programs are of the same type. But when
a compatibility check is made in a program that performs a tail-call,
the is_valid_tail_call() function is called (if available) to allow
the target type to determine whether it can handle the conversion of
a context from the source type to the target type. If the function is
not implemented by the program type, casting is denied.

During execution, the convert_ctx() function is called (if available)
to perform the conversion of the current context to the context that the
target type expects. Since the program type of the executing BPF program
is not explicitly known during execution, the verifier inserts an
instruction right before the tail-call to assign the current BPF program
type to R4.

The interpreter calls convert_ctx() using the program type in R4 as
source program type, the program type associated with the program array
as target program type, and the context as provided in R1.

A helper (finalize_context) is added to allow tail called programs to
perform context setup based on information that is passed in from the
calling program by means of a map that is indexed by CPU id. The actual
content of the map is defined by the BPF program type implementation
for the program type that is being called.

The bpf_prog_types array is now being exposed to the rest of the BPF
code (where before it was local to just the syscall handling) because
the is_valid_tail_call() and convert_ctx() operations need to be
accessible.

There is no noticeable effect on BPF program types that do not implement
this new feature.

A JIT implementation is not available yet in this first iteration.

v2: Fixed compilation when CONFIG_BPF_SYSCALL=n.
Fixed casting issue on platforms with 32-bit pointers.

v3: Renamed the new program type operations to be more descriptive.
Added finalize_context() helper.

Signed-off-by: Kris Van Hees <kris.van.hees@xxxxxxxxxx>
Reviewed-by: Nick Alcock <nick.alcock@xxxxxxxxxx>
---
include/linux/bpf.h | 3 +++
include/uapi/linux/bpf.h | 11 ++++++++-
kernel/bpf/core.c | 29 ++++++++++++++++++++++-
kernel/bpf/syscall.c | 2 +-
kernel/bpf/verifier.c | 16 +++++++++----
tools/include/uapi/linux/bpf.h | 11 ++++++++-
tools/testing/selftests/bpf/bpf_helpers.h | 2 ++
7 files changed, 66 insertions(+), 8 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 59631dd0777c..7a40a3cd7ff2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -294,6 +294,8 @@ bpf_ctx_record_field_size(struct bpf_insn_access_aux *aux, u32 size)
struct bpf_prog_ops {
int (*test_run)(struct bpf_prog *prog, const union bpf_attr *kattr,
union bpf_attr __user *uattr);
+ bool (*is_valid_tail_call)(enum bpf_prog_type stype);
+ void *(*convert_ctx)(enum bpf_prog_type stype, void *ctx);
};

struct bpf_verifier_ops {
@@ -571,6 +573,7 @@ extern const struct file_operations bpf_prog_fops;
#undef BPF_PROG_TYPE
#undef BPF_MAP_TYPE

+extern const struct bpf_prog_ops * const bpf_prog_types[];
extern const struct bpf_prog_ops bpf_offload_prog_ops;
extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
extern const struct bpf_verifier_ops xdp_analyzer_ops;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 63e0cf66f01a..61abe6b56948 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2672,6 +2672,14 @@ union bpf_attr {
* 0 on success.
*
* **-ENOENT** if the bpf-local-storage cannot be found.
+ *
+ * int bpf_finalize_context(void *ctx, struct bpf_map *map)
+ * Description
+ * Perform any final context setup after a tail call took
+ * place from another BPF program type into a program of
+ * the implementing program type.
+ * Return
+ * 0 on success, or a negative error in case of failure.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2782,7 +2790,8 @@ union bpf_attr {
FN(strtol), \
FN(strtoul), \
FN(sk_storage_get), \
- FN(sk_storage_delete),
+ FN(sk_storage_delete), \
+ FN(finalize_context),

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 242a643af82f..225b1be766b0 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1456,10 +1456,12 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
CONT;

JMP_TAIL_CALL: {
+ void *ctx = (void *) (unsigned long) BPF_R1;
struct bpf_map *map = (struct bpf_map *) (unsigned long) BPF_R2;
struct bpf_array *array = container_of(map, struct bpf_array, map);
struct bpf_prog *prog;
u32 index = BPF_R3;
+ u32 type = BPF_R4;

if (unlikely(index >= array->map.max_entries))
goto out;
@@ -1471,6 +1473,13 @@ static u64 ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u64 *stack)
prog = READ_ONCE(array->ptrs[index]);
if (!prog)
goto out;
+ if (prog->aux->ops->convert_ctx) {
+ ctx = prog->aux->ops->convert_ctx(type, ctx);
+ if (!ctx)
+ goto out;
+
+ BPF_R1 = (u64) (uintptr_t) ctx;
+ }

/* ARG1 at this point is guaranteed to point to CTX from
* the verifier side due to the fact that the tail call is
@@ -1667,6 +1676,23 @@ bool bpf_prog_array_compatible(struct bpf_array *array,
array->owner_jited == fp->jited;
}

+bool bpf_prog_array_valid_tail_call(struct bpf_array *array,
+ const struct bpf_prog *fp)
+{
+#ifdef CONFIG_BPF_SYSCALL
+ const struct bpf_prog_ops *ops;
+
+ if (array->owner_jited != fp->jited)
+ return false;
+
+ ops = bpf_prog_types[array->owner_prog_type];
+ if (ops->is_valid_tail_call)
+ return ops->is_valid_tail_call(fp->type);
+#endif
+
+ return false;
+}
+
static int bpf_check_tail_call(const struct bpf_prog *fp)
{
struct bpf_prog_aux *aux = fp->aux;
@@ -1680,7 +1706,8 @@ static int bpf_check_tail_call(const struct bpf_prog *fp)
continue;

array = container_of(map, struct bpf_array, map);
- if (!bpf_prog_array_compatible(array, fp))
+ if (!bpf_prog_array_compatible(array, fp) &&
+ !bpf_prog_array_valid_tail_call(array, fp))
return -EINVAL;
}

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ad3ccf82f31d..f76fd30ad372 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1179,7 +1179,7 @@ static int map_freeze(const union bpf_attr *attr)
return err;
}

-static const struct bpf_prog_ops * const bpf_prog_types[] = {
+const struct bpf_prog_ops * const bpf_prog_types[] = {
#define BPF_PROG_TYPE(_id, _name) \
[_id] = & _name ## _prog_ops,
#define BPF_MAP_TYPE(_id, _ops)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 95f9354495ad..f9e5536fd1af 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7982,9 +7982,10 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
insn->imm = 0;
insn->code = BPF_JMP | BPF_TAIL_CALL;

+ cnt = 0;
aux = &env->insn_aux_data[i + delta];
if (!bpf_map_ptr_unpriv(aux))
- continue;
+ goto privileged;

/* instead of changing every JIT dealing with tail_call
* emit two extra insns:
@@ -7999,13 +8000,20 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)

map_ptr = BPF_MAP_PTR(aux->map_state);
insn_buf[0] = BPF_JMP_IMM(BPF_JGE, BPF_REG_3,
- map_ptr->max_entries, 2);
+ map_ptr->max_entries, 3);
insn_buf[1] = BPF_ALU32_IMM(BPF_AND, BPF_REG_3,
container_of(map_ptr,
struct bpf_array,
map)->index_mask);
- insn_buf[2] = *insn;
- cnt = 3;
+ cnt = 2;
+
+privileged:
+ /* store the BPF program type of the current program in
+ * R4 so it is known in case this tail call requires
+ * casting the context to a different program type
+ */
+ insn_buf[cnt++] = BPF_MOV64_IMM(BPF_REG_4, prog->type);
+ insn_buf[cnt++] = *insn;
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
if (!new_prog)
return -ENOMEM;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 63e0cf66f01a..61abe6b56948 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2672,6 +2672,14 @@ union bpf_attr {
* 0 on success.
*
* **-ENOENT** if the bpf-local-storage cannot be found.
+ *
+ * int bpf_finalize_context(void *ctx, struct bpf_map *map)
+ * Description
+ * Perform any final context setup after a tail call took
+ * place from another BPF program type into a program of
+ * the implementing program type.
+ * Return
+ * 0 on success, or a negative error in case of failure.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2782,7 +2790,8 @@ union bpf_attr {
FN(strtol), \
FN(strtoul), \
FN(sk_storage_get), \
- FN(sk_storage_delete),
+ FN(sk_storage_delete), \
+ FN(finalize_context),

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 6e80b66d7fb1..d98a62b3b56c 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -216,6 +216,8 @@ static void *(*bpf_sk_storage_get)(void *map, struct bpf_sock *sk,
(void *) BPF_FUNC_sk_storage_get;
static int (*bpf_sk_storage_delete)(void *map, struct bpf_sock *sk) =
(void *)BPF_FUNC_sk_storage_delete;
+static int (*bpf_finalize_context)(void *ctx, void *map) =
+ (void *) BPF_FUNC_finalize_context;

/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
--
2.20.1

Next message: Kris Van Hees: "[RFC PATCH 05/11] trace: update Kconfig and Makefile to include DTrace"
Previous message: Kees Cook: "Re: [PATCH v3] kernel: fix typos and some coding style in comments"
In reply to: Kris Van Hees: "[RFC PATCH 11/11] dtrace: make use of writable buffers in BPF"
Next in thread: Kris Van Hees: "[RFC PATCH 05/11] trace: update Kconfig and Makefile to include DTrace"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]