On Wed, Aug 21, 2024 at 9:46 AM 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, 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.
>
> 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>
> ---
> tools/lib/bpf/relo_core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
> index 63a4d5ad12d1..a04724831ebc 100644
> --- a/tools/lib/bpf/relo_core.c
> +++ b/tools/lib/bpf/relo_core.c
> @@ -1297,6 +1297,11 @@ int bpf_core_calc_relo_insn(const char *prog_name,
>
> local_id = relo->type_id;
> local_type = btf_type_by_id(local_btf, local_id);
> + if (!local_type) {
This is a meaningless check at least for libbpf's implementation of
btf_type_by_id(), it never returns NULL. Commit you point to in Fixes
tag clearly states the differences.
So you'd need to validate local_id directly against number of types in
local_btf.
pw-bot: cr
> + pr_warn("prog '%s': relo #%d: bad type id %u\n",
nit: this part of CO-RE-related code normally uses [%u] "syntax" to
point to BTF type IDs, please adjust for consistency
> + prog_name, relo_idx, local_id);
> + return -EINVAL;
> + }
> local_name = btf__name_by_offset(local_btf, local_type->name_off);
> if (!local_name)
> return -EINVAL;
> --
> 2.45.2
>
