Re: KASAN: null-ptr-deref in bpf_core_calc_relo_insn

[Eduard Zingerman <eddyz87@xxxxxxxxx>]

On Tue, 2024-08-20 at 18:33 -0700, Eduard Zingerman wrote:
> On Tue, 2024-08-20 at 17:21 +0800, Liu RuiTong wrote:
> 
> [...]
> 
> > bpf_core_calc_relo_insn+311            <bpf_core_calc_relo_insn+311>
> > ─────────────────────────────────────────────────────────────────────────────────────────────[
> > SOURCE (CODE) ]──────────────────────────────────────────────────────────────────────────────────────────────
> > In file: /home/ubuntu/fuzz/linux-6.11-rc4/tools/lib/bpf/relo_core.c:1300
> >    1295         char spec_buf[256];
> >    1296         int i, j, err;
> >    1297
> >    1298         local_id = relo->type_id;
> >    1299         local_type = btf_type_by_id(local_btf, local_id);
> >  ► 1300         local_name = btf__name_by_offset(local_btf,
> > local_type->name_off);
> 
> Hi Liu,
> 
> Thank you for the report, I can reproduce the issue.
> Will comment later today.

Hi Liu,

Your analysis is correct, the issue is caused by a missing null
pointer check for 'local_type'.

I was curious why the attached test case does not cause null pointer
exception every time, but then I realized that this is because of the
sequence of BPF commands it issues (each in separate thread):
1. Create BTF, wait for completion;
2. Load BPF program, do not wait for completion;
3. Rewrite memory region passed to load BPF command as bpf_attr to
   reuse it for another system call (actual call is map update, but
   that does not matter).

From time to time steps (2) and (3) would run concurrently and user
space memory chunk passed to kernel in (2) would be updated to make
relocation data invalid.

I attach a simplified test case, will post a fix to bpf mailing list
shortly.

// SPDX-License-Identifier: GPL-2.0

/* Test cases that can't load programs using libbpf and need direct
 * BPF syscall access
 */

#include <sys/syscall.h>
#include <bpf/libbpf.h>
#include <bpf/btf.h>

#include "test_progs.h"
#include "test_btf.h"
#include "bpf/libbpf_internal.h"

static char log[16 * 1024];

static void test_bad_local_id()
{
	struct test_btf {
		struct btf_header hdr;
		__u32 types[15];
		char strings[128];
	} raw_btf = {
		.hdr = {
			.magic = BTF_MAGIC,
			.version = BTF_VERSION,
			.hdr_len = sizeof(struct btf_header),
			.type_off = 0,
			.type_len = sizeof(raw_btf.types),
			.str_off = offsetof(struct test_btf, strings) - offsetof(struct test_btf, types),
			.str_len = sizeof(raw_btf.strings),
		},
		.types = {
			BTF_PTR_ENC(0),					/* [1] void* */
			BTF_TYPE_INT_ENC(1, BTF_INT_SIGNED, 0, 32, 4),	/* [2] int */
			BTF_FUNC_PROTO_ENC(2, 1),			/* [3] int (*)(void*) */
			BTF_FUNC_PROTO_ARG_ENC(8, 1),
			BTF_FUNC_ENC(8, 3)				/* [4] FUNC 'foo' type_id=2 */
		},
		.strings = "\0int\0 0\0foo\0"
	};
	__u32 log_level = 1 | 2 | 4;
	LIBBPF_OPTS(bpf_btf_load_opts, opts,
		    .log_buf = log,
		    .log_size = sizeof(log),
		    .log_level = log_level,
	);
	struct bpf_insn insns[] = {
		BPF_ALU64_IMM(BPF_MOV, BPF_REG_0, 0),
		BPF_EXIT_INSN(),
	};
	struct bpf_func_info funcs[] = {
		{
			.insn_off = 0,
			.type_id = 4,
		}
	};
	struct bpf_core_relo relos[] = {
		{
			.insn_off = 0,		/* patch first instruction (r0 = 0) */
			.type_id = 100500,	/* !!! this type id does not exist */
			.access_str_off = 6,	/* offset of "0" */
			.kind = BPF_CORE_TYPE_ID_LOCAL,
		}
	};
	union bpf_attr attr = {
		.prog_name = "foo",
		.prog_type = BPF_TRACE_RAW_TP,
		.license = (__u64)"GPL",
		.insns = (__u64)&insns,
		.insn_cnt = sizeof(insns) / sizeof(*insns),
		.log_buf = (__u64)log,
		.log_size = sizeof(log),
		.log_level = log_level,
		.func_info = (__u64)funcs,
		.func_info_cnt = sizeof(funcs) / sizeof(*funcs),
		.func_info_rec_size = sizeof(*funcs),
		.core_relos = (__u64)relos,
		.core_relo_cnt = sizeof(relos) / sizeof(*relos),
		.core_relo_rec_size = sizeof(*relos),
	};
	int saved_errno;
	int prog_fd = -1;
	int btf_fd = -1;

	btf_fd = bpf_btf_load(&raw_btf, sizeof(raw_btf), &opts);
	saved_errno = errno;
	if (btf_fd < 0 || env.verbosity > VERBOSE_NORMAL) {
		printf("-------- BTF load log start --------\n");
		printf("%s", log);
		printf("-------- BTF load log end ----------\n");
	}
	if (btf_fd < 0) {
		PRINT_FAIL("bpf_btf_load() failed, errno=%d\n", saved_errno);
		return;
	}

	memset(log, 0, sizeof(log));
	attr.prog_btf_fd = btf_fd;
	prog_fd = sys_bpf_prog_load(&attr, sizeof(attr), 1);
	saved_errno = errno;
	if (prog_fd < 0 || env.verbosity > VERBOSE_NORMAL) {
		printf("-------- program load log start --------\n");
		printf("%s", log);
		printf("-------- program load log end ----------\n");
	}
	if (prog_fd > 0) {
		PRINT_FAIL("sys_bpf_prog_load() expected to fail\n");
		goto out;
	}
	ASSERT_HAS_SUBSTR(log, "relo #0: bad type id 100500", "program load log");

out:
	close(prog_fd);
	close(btf_fd);
}

void test_core_reloc_raw(void)
{
	if (test__start_subtest("bad_local_id"))
		test_bad_local_id();
}