Commit b78b34c6 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf: fix NULL dereference during extable search' 



Krister Johansen says:

====================
Hi,
Enclosed are a pair of patches for an oops that can occur if an exception is
generated while a bpf subprogram is running.  One of the bpf_prog_aux entries
for the subprograms are missing an extable.  This can lead to an exception that
would otherwise be handled turning into a NULL pointer bug.

These changes were tested via the verifier and progs selftests and no
regressions were observed.

Changes from v4:
- Ensure that num_exentries is copied to prog->aux from func[0] (Feedback from
  Ilya Leoshkevich)

Changes from v3:
- Selftest style fixups (Feedback from Yonghong Song)
- Selftest needs to assert that test bpf program executed (Feedback from
  Yonghong Song)
- Selftest should combine open and load using open_and_load (Feedback from
  Yonghong Song)

Changes from v2:
- Insert only the main program's kallsyms (Feedback from Yonghong Song and
  Alexei Starovoitov)
- Selftest should use ASSERT instead of CHECK (Feedback from Yonghong Song)
- Selftest needs some cleanup (Feedback from Yonghong Song)
- Switch patch order (Feedback from Alexei Starovoitov)

Changes from v1:
- Add a selftest (Feedback From Alexei Starovoitov)
- Move to a 1-line verifier change instead of searching multiple extables
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents ad96f1c9 84a62b44

kernel/bpf/verifier.c

+5 −2
Original line number Diff line number Diff line
@@ -17217,9 +17217,10 @@ static int jit_subprogs(struct bpf_verifier_env *env) 
	}









	/* finally lock prog and jit images for all functions and









	 * populate kallsysm









	 * populate kallsysm. Begin at the first subprogram, since









	 * bpf_prog_load will add the kallsyms for the main program.









	 */









	for (i = 0; i < env->subprog_cnt; i++) {









	for (i = 1; i < env->subprog_cnt; i++) {









		bpf_prog_lock_ro(func[i]);









		bpf_prog_kallsyms_add(func[i]);









	}








@@ -17245,6 +17246,8 @@ static int jit_subprogs(struct bpf_verifier_env *env)







	prog->jited = 1;









	prog->bpf_func = func[0]->bpf_func;









	prog->jited_len = func[0]->jited_len;









	prog->aux->extable = func[0]->aux->extable;









	prog->aux->num_exentries = func[0]->aux->num_exentries;









	prog->aux->func = func;









	prog->aux->func_cnt = env->subprog_cnt;









	bpf_prog_jit_attempt_done(prog);
































tools/testing/selftests/bpf/prog_tests/subprogs_extable.c
0 → 100644



+29
−0






























Original line number
Diff line number
Diff line









// SPDX-License-Identifier: GPL-2.0



















#include <test_progs.h>









#include "test_subprogs_extable.skel.h"



















void test_subprogs_extable(void)









{









	const int read_sz = 456;









	struct test_subprogs_extable *skel;









	int err;



















	skel = test_subprogs_extable__open_and_load();









	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))









		return;



















	err = test_subprogs_extable__attach(skel);









	if (!ASSERT_OK(err, "skel_attach"))









		goto cleanup;



















	/* trigger tracepoint */









	ASSERT_OK(trigger_module_test_read(read_sz), "trigger_read");



















	ASSERT_NEQ(skel->bss->triggered, 0, "verify at least one program ran");



















	test_subprogs_extable__detach(skel);



















cleanup:









	test_subprogs_extable__destroy(skel);









}
























tools/testing/selftests/bpf/progs/test_subprogs_extable.c
0 → 100644



+51
−0






























Original line number
Diff line number
Diff line









// SPDX-License-Identifier: GPL-2.0



















#include "vmlinux.h"









#include <bpf/bpf_helpers.h>









#include <bpf/bpf_tracing.h>



















struct {









	__uint(type, BPF_MAP_TYPE_ARRAY);









	__uint(max_entries, 8);









	__type(key, __u32);









	__type(value, __u64);









} test_array SEC(".maps");



















unsigned int triggered;



















static __u64 test_cb(struct bpf_map *map, __u32 *key, __u64 *val, void *data)









{









	return 1;









}



















SEC("fexit/bpf_testmod_return_ptr")









int BPF_PROG(handle_fexit_ret_subprogs, int arg, struct file *ret)









{









	*(volatile long *)ret;









	*(volatile int *)&ret->f_mode;









	bpf_for_each_map_elem(&test_array, test_cb, NULL, 0);









	triggered++;









	return 0;









}



















SEC("fexit/bpf_testmod_return_ptr")









int BPF_PROG(handle_fexit_ret_subprogs2, int arg, struct file *ret)









{









	*(volatile long *)ret;









	*(volatile int *)&ret->f_mode;









	bpf_for_each_map_elem(&test_array, test_cb, NULL, 0);









	triggered++;









	return 0;









}



















SEC("fexit/bpf_testmod_return_ptr")









int BPF_PROG(handle_fexit_ret_subprogs3, int arg, struct file *ret)









{









	*(volatile long *)ret;









	*(volatile int *)&ret->f_mode;









	bpf_for_each_map_elem(&test_array, test_cb, NULL, 0);









	triggered++;









	return 0;









}



















char _license[] SEC("license") = "GPL";