On Wed, Aug 21, 2024 at 5:18 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> In case of malformed relocation record of kind BPF_CORE_TYPE_ID_LOCAL
> referencing a non-existing BTF type, function bpf_core_calc_relo_insn
> would cause a null pointer deference.
>
> Fix this by adding a proper check upper in call stack, as malformed
> relocation records could be passed from user space.
>
> Simplest reproducer is a program:
>
> r0 = 0
> exit
>
> With a single relocation record:
>
> .insn_off = 0, /* patch first instruction */
> .type_id = 100500, /* this type id does not exist */
> .access_str_off = 6, /* offset of string "0" */
> .kind = BPF_CORE_TYPE_ID_LOCAL,
>
> See the link for original reproducer or next commit for a test case.
>
> Fixes: 74753e1462e7 ("libbpf: Replace btf__type_by_id() with btf_type_by_id().")
> Reported-by: Liu RuiTong <cnitlrt@xxxxxxxxx>
> Closes: https://lore.kernel.org/bpf/CAK55_s6do7C+DVwbwY_7nKfUz0YLDoiA1v6X3Y9+p0sWzipFSA@xxxxxxxxxxxxxx/
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> kernel/bpf/btf.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
LGTM
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index b12db397303e..e38e770a6945 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -8888,6 +8888,7 @@ int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo,
> struct bpf_core_cand_list cands = {};
> struct bpf_core_relo_res targ_res;
> struct bpf_core_spec *specs;
> + const struct btf_type *type;
> int err;
>
> /* ~4k of temp memory necessary to convert LLVM spec like "0:1:0:5"
> @@ -8897,6 +8898,13 @@ int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo,
> if (!specs)
> return -ENOMEM;
>
> + type = btf_type_by_id(ctx->btf, relo->type_id);
> + if (!type) {
> + bpf_log(ctx->log, "relo #%u: bad type id %u\n",
> + relo_idx, relo->type_id);
> + return -EINVAL;
> + }
> +
> if (need_cands) {
> struct bpf_cand_cache *cc;
> int i;
> --
> 2.45.2
>
