| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEf4BzbmE_ADjjhu-+mLkQN_5HD6Azhrb-VuprAaoqxP1bb3OQ@mail.gmail.com>
Date: Fri, 11 Oct 2019 12:02:50 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Alexei Starovoitov <ast@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>,
Daniel Borkmann <daniel@...earbox.net>, x86@...nel.org,
Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v2 bpf-next 10/12] bpf: check types of arguments passed
into helpers
On Wed, Oct 9, 2019 at 9:15 PM Alexei Starovoitov <ast@...nel.org> wrote:
>
> Introduce new helper that reuses existing skb perf_event output
> implementation, but can be called from raw_tracepoint programs
> that receive 'struct sk_buff *' as tracepoint argument or
> can walk other kernel data structures to skb pointer.
>
> In order to do that teach verifier to resolve true C types
> of bpf helpers into in-kernel BTF ids.
> The type of kernel pointer passed by raw tracepoint into bpf
> program will be tracked by the verifier all the way until
> it's passed into helper function.
> For example:
> kfree_skb() kernel function calls trace_kfree_skb(skb, loc);
> bpf programs receives that skb pointer and may eventually
> pass it into bpf_skb_output() bpf helper which in-kernel is
> implemented via bpf_skb_event_output() kernel function.
> Its first argument in the kernel is 'struct sk_buff *'.
> The verifier makes sure that types match all the way.
>
> Signed-off-by: Alexei Starovoitov <ast@...nel.org>
> ---
> include/linux/bpf.h | 18 ++++++---
> include/uapi/linux/bpf.h | 27 +++++++++++++-
> kernel/bpf/btf.c | 68 ++++++++++++++++++++++++++++++++++
> kernel/bpf/verifier.c | 44 ++++++++++++++--------
> kernel/trace/bpf_trace.c | 4 ++
> net/core/filter.c | 15 +++++++-
> tools/include/uapi/linux/bpf.h | 27 +++++++++++++-
> 7 files changed, 180 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 6edfe50f1c2c..d3df073f374a 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -213,6 +213,7 @@ enum bpf_arg_type {
> ARG_PTR_TO_INT, /* pointer to int */
> ARG_PTR_TO_LONG, /* pointer to long */
> ARG_PTR_TO_SOCKET, /* pointer to bpf_sock (fullsock) */
> + ARG_PTR_TO_BTF_ID, /* pointer to in-kernel struct */
> };
>
> /* type of values returned from helper functions */
> @@ -235,11 +236,17 @@ struct bpf_func_proto {
> bool gpl_only;
> bool pkt_access;
> enum bpf_return_type ret_type;
> - enum bpf_arg_type arg1_type;
> - enum bpf_arg_type arg2_type;
> - enum bpf_arg_type arg3_type;
> - enum bpf_arg_type arg4_type;
> - enum bpf_arg_type arg5_type;
> + union {
> + struct {
> + enum bpf_arg_type arg1_type;
> + enum bpf_arg_type arg2_type;
> + enum bpf_arg_type arg3_type;
> + enum bpf_arg_type arg4_type;
> + enum bpf_arg_type arg5_type;
> + };
> + enum bpf_arg_type arg_type[5];
> + };
> + u32 *btf_id; /* BTF ids of arguments */
are you trying to save memory with this? otherwise not sure why it's
not just `u32 btf_id[5]`? Even in that case it will save at most 12
bytes (and I haven't even check alignment padding and stuff). So
doesn't seem worth it?
> };
>
> /* bpf_context is intentionally undefined structure. Pointer to bpf_context is
> @@ -765,6 +772,7 @@ int btf_struct_access(struct bpf_verifier_log *log,
> const struct btf_type *t, int off, int size,
> enum bpf_access_type atype,
> u32 *next_btf_id);
> +u32 btf_resolve_helper_id(struct bpf_verifier_log *log, void *, int);
>
> #else /* !CONFIG_BPF_SYSCALL */
> static inline struct bpf_prog *bpf_prog_get(u32 ufd)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 3bb2cd1de341..b0454440186f 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2751,6 +2751,30 @@ union bpf_attr {
> * **-EOPNOTSUPP** kernel configuration does not enable SYN cookies
> *
> * **-EPROTONOSUPPORT** IP packet version is not 4 or 6
> + *
> + * int bpf_skb_output(void *ctx, struct bpf_map *map, u64 flags, void *data, u64 size)
> + * Description
> + * Write raw *data* blob into a special BPF perf event held by
> + * *map* of type **BPF_MAP_TYPE_PERF_EVENT_ARRAY**. This perf
> + * event must have the following attributes: **PERF_SAMPLE_RAW**
> + * as **sample_type**, **PERF_TYPE_SOFTWARE** as **type**, and
> + * **PERF_COUNT_SW_BPF_OUTPUT** as **config**.
> + *
> + * The *flags* are used to indicate the index in *map* for which
> + * the value must be put, masked with **BPF_F_INDEX_MASK**.
> + * Alternatively, *flags* can be set to **BPF_F_CURRENT_CPU**
> + * to indicate that the index of the current CPU core should be
> + * used.
> + *
> + * The value to write, of *size*, is passed through eBPF stack and
> + * pointed by *data*.
typo? pointed __to__ by *data*?
> + *
> + * *ctx* is a pointer to in-kernel sutrct sk_buff.
> + *
> + * This helper is similar to **bpf_perf_event_output**\ () but
> + * restricted to raw_tracepoint bpf programs.
nit: with BTF type tracking enabled?
> + * Return
> + * 0 on success, or a negative error in case of failure.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2863,7 +2887,8 @@ union bpf_attr {
> FN(sk_storage_get), \
> FN(sk_storage_delete), \
> FN(send_signal), \
> - FN(tcp_gen_syncookie),
> + FN(tcp_gen_syncookie), \
> + FN(skb_output),
>
[...]
> @@ -4072,21 +4091,16 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>
> meta.func_id = func_id;
> /* check args */
> - err = check_func_arg(env, BPF_REG_1, fn->arg1_type, &meta);
> - if (err)
> - return err;
> - err = check_func_arg(env, BPF_REG_2, fn->arg2_type, &meta);
> - if (err)
> - return err;
> - err = check_func_arg(env, BPF_REG_3, fn->arg3_type, &meta);
> - if (err)
> - return err;
> - err = check_func_arg(env, BPF_REG_4, fn->arg4_type, &meta);
> - if (err)
> - return err;
> - err = check_func_arg(env, BPF_REG_5, fn->arg5_type, &meta);
> - if (err)
> - return err;
> + for (i = 0; i < 5; i++) {
> + if (fn->arg_type[i] == ARG_PTR_TO_BTF_ID) {
> + if (!fn->btf_id[i])
> + fn->btf_id[i] = btf_resolve_helper_id(&env->log, fn->func, 0);
bug: 0 -> i :)
> + meta.btf_id = fn->btf_id[i];
> + }
> + err = check_func_arg(env, BPF_REG_1 + i, fn->arg_type[i], &meta);
> + if (err)
> + return err;
> + }
>
[...]
Powered by blists - more mailing lists