[bpf-next,v2,1/2] bpf: correctly handle malformed BPF_CORE_TYPE_ID_LOCAL relos

Commit Message

Eduard Zingerman Aug. 22, 2024, 12:18 a.m. UTC

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 <[e-mail]>
Closes: https://lore.kernel.org/bpf/[e-mail]/
Signed-off-by: Eduard Zingerman <[e-mail]>
---
 kernel/bpf/btf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Andrii Nakryiko Aug. 22, 2024, 4:29 a.m. UTC | #1

On Wed, Aug 21, 2024 at 5:18 PM Eduard Zingerman <[e-mail]> 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 <[e-mail]>
> Closes: https://lore.kernel.org/bpf/[e-mail]/
> Signed-off-by: Eduard Zingerman <[e-mail]>
> ---
>  kernel/bpf/btf.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>

LGTM

Acked-by: Andrii Nakryiko <[e-mail]>

> 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
>

Patch

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;