Re: [PATCH v5 bpf-next 05/17] bpf: Pass a set of bpf_core_relo-s to prog_load command.

[Jakub Sitnicki <jakub@cloudflare.com>]

Hi Alexei,

On Wed, Dec 01, 2021 at 10:10 AM -08, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> struct bpf_core_relo is generated by llvm and processed by libbpf.
> It's a de-facto uapi.
> With CO-RE in the kernel the struct bpf_core_relo becomes uapi de-jure.
> Add an ability to pass a set of 'struct bpf_core_relo' to prog_load command
> and let the kernel perform CO-RE relocations.
>
> Note the struct bpf_line_info and struct bpf_func_info have the same
> layout when passed from LLVM to libbpf and from libbpf to the kernel
> except "insn_off" fields means "byte offset" when LLVM generates it.
> Then libbpf converts it to "insn index" to pass to the kernel.
> The struct bpf_core_relo's "insn_off" field is always "byte offset".
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  include/linux/bpf.h            |  8 ++++
>  include/uapi/linux/bpf.h       | 59 +++++++++++++++++++++++++-
>  kernel/bpf/btf.c               |  6 +++
>  kernel/bpf/syscall.c           |  2 +-
>  kernel/bpf/verifier.c          | 76 ++++++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h | 59 +++++++++++++++++++++++++-
>  tools/lib/bpf/relo_core.h      | 53 ------------------------
>  7 files changed, 207 insertions(+), 56 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index cad0829710be..8bbf08fbab66 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1732,6 +1732,14 @@ bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog);
>  const struct btf_func_model *
>  bpf_jit_find_kfunc_model(const struct bpf_prog *prog,
>  			 const struct bpf_insn *insn);
> +struct bpf_core_ctx {
> +	struct bpf_verifier_log *log;
> +	const struct btf *btf;
> +};
> +
> +int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo,
> +		   int relo_idx, void *insn);
> +
>  #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 9e66b1880020..c26871263f1f 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1342,8 +1342,10 @@ union bpf_attr {
>  			/* or valid module BTF object fd or 0 to attach to vmlinux */
>  			__u32		attach_btf_obj_fd;
>  		};
> -		__u32		:32;		/* pad */
> +		__u32		core_relo_cnt;	/* number of bpf_core_relo */
>  		__aligned_u64	fd_array;	/* array of FDs */
> +		__aligned_u64	core_relos;
> +		__u32		core_relo_rec_size; /* sizeof(struct bpf_core_relo) */
>  	};
>  
>  	struct { /* anonymous struct used by BPF_OBJ_* commands */

I think I've spotted a breakage.

Plugging the 4 byte hole with core_relo_cnt means that programs built
against < v5.17 headers pass garbage as core_relo_cnt value.

That in turn makes check_core_relo() fail with -EINVAL, which fails
PROG_LOAD.

[...]