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

Merge branch 'Fix for crash due to overwrite in copy_map_value' 

Kumar Kartikeya says:

====================

A fix for an oversight in copy_map_value that leads to kernel crash.

Also, a question for BPF developers:
It seems in arraymap.c, we always do check_and_free_timer_in_array after we do
copy_map_value in map_update_elem callback, but the same is not done for
hashtab.c. Is there a specific reason for this difference in behavior, or did I
miss that it happens for hashtab.c as well?

Changlog:
---------
v1 -> v2:
v1: https://lore.kernel.org/bpf/20220209051113.870717-1-memxor@gmail.com



 * Fix build error for selftests patch due to missing SYS_PREFIX in bpf tree
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 4a11678f a7e75016

include/linux/bpf.h

+2 −1
Original line number Diff line number Diff line
@@ -224,7 +224,8 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src) 
	if (unlikely(map_value_has_spin_lock(map))) {

		s_off = map->spin_lock_off;

		s_sz = sizeof(struct bpf_spin_lock);

	} else if (unlikely(map_value_has_timer(map))) {

	}

	if (unlikely(map_value_has_timer(map))) {

		t_off = map->timer_off;

		t_sz = sizeof(struct bpf_timer);

	}

tools/testing/selftests/bpf/prog_tests/timer_crash.c

0 → 100644
+32 −0
Original line number Diff line number Diff line 
// SPDX-License-Identifier: GPL-2.0

#include <test_progs.h>

#include "timer_crash.skel.h"



enum {

	MODE_ARRAY,

	MODE_HASH,

};



static void test_timer_crash_mode(int mode)

{

	struct timer_crash *skel;



	skel = timer_crash__open_and_load();

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

		return;

	skel->bss->pid = getpid();

	skel->bss->crash_map = mode;

	if (!ASSERT_OK(timer_crash__attach(skel), "timer_crash__attach"))

		goto end;

	usleep(1);

end:

	timer_crash__destroy(skel);

}



void test_timer_crash(void)

{

	if (test__start_subtest("array"))

		test_timer_crash_mode(MODE_ARRAY);

	if (test__start_subtest("hash"))

		test_timer_crash_mode(MODE_HASH);

}

tools/testing/selftests/bpf/progs/timer_crash.c

0 → 100644
+54 −0
Original line number Diff line number Diff line 
// SPDX-License-Identifier: GPL-2.0



#include <vmlinux.h>

#include <bpf/bpf_tracing.h>

#include <bpf/bpf_helpers.h>



struct map_elem {

	struct bpf_timer timer;

	struct bpf_spin_lock lock;

};



struct {

	__uint(type, BPF_MAP_TYPE_ARRAY);

	__uint(max_entries, 1);

	__type(key, int);

	__type(value, struct map_elem);

} amap SEC(".maps");



struct {

	__uint(type, BPF_MAP_TYPE_HASH);

	__uint(max_entries, 1);

	__type(key, int);

	__type(value, struct map_elem);

} hmap SEC(".maps");



int pid = 0;

int crash_map = 0; /* 0 for amap, 1 for hmap */



SEC("fentry/do_nanosleep")

int sys_enter(void *ctx)

{

	struct map_elem *e, value = {};

	void *map = crash_map ? (void *)&hmap : (void *)&amap;



	if (bpf_get_current_task_btf()->tgid != pid)

		return 0;



	*(void **)&value = (void *)0xdeadcaf3;



	bpf_map_update_elem(map, &(int){0}, &value, 0);

	/* For array map, doing bpf_map_update_elem will do a

	 * check_and_free_timer_in_array, which will trigger the crash if timer

	 * pointer was overwritten, for hmap we need to use bpf_timer_cancel.

	 */

	if (crash_map == 1) {

		e = bpf_map_lookup_elem(map, &(int){0});

		if (!e)

			return 0;

		bpf_timer_cancel(&e->timer);

	}

	return 0;

}



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