bpf: Prevent mmap()'ing read-only maps as writable

As discussed in [0], it's dangerous to allow mapping BPF map, that's meant to be frozen and is read-only on BPF program side, because that allows user-space to actually store a writable view to the page even after it is frozen. This is exacerbated by BPF verifier making a strong assumption that contents of such frozen map will remain unchanged. To prevent this, disallow mapping BPF_F_RDONLY_PROG mmap()'able BPF maps as writable, ever. [0] https://lore.kernel.org/bpf/CAEf4BzYGWYhXdp6BJ7_=9OQPJxQpgug080MMjdSB72i9R+5c6g@mail.gmail.com/

Andrii Nakryiko committed 6 years ago

Commit dfeb3761 parent 0550cfe3 files · +34 −4Browse files

Unified · Split

3 files

kernel/bpf/syscall.c +14 −3
tools/testing/selftests/bpf/prog_tests/mmap.c +12 −1
tools/testing/selftests/bpf/progs/test_mmap.c +8 −0

kernel/bpf/syscall.c+14−3

@@ -623,9 +623,20 @@ static int bpf_map_mmap(struct file filp, struct vm_area_struct vma) · 622 unmodified lines
623	623
624	624	mutex_lock(&map->freeze_mutex);
625	625
626		− if ((vma->vm_flags & VM_WRITE) && map->frozen) {
627		− err = -EPERM;
628		− goto out;
	626	+ if (vma->vm_flags & VM_WRITE) {
	627	+ if (map->frozen) {
	628	+ err = -EPERM;
	629	+ goto out;
	630	+ }
	631	+ /* map is meant to be read-only, so do not allow mapping as
	632	+ * writable, because it's possible to leak a writable page
	633	+ * reference and allows user-space to still modify it after
	634	+ * freezing, while verifier will assume contents do not change
	635	+ */
	636	+ if (map->map_flags & BPF_F_RDONLY_PROG) {
	637	+ err = -EACCES;
	638	+ goto out;
	639	+ }
629	640	}
630	641
631	642	/* set default open/close callbacks */

tools/testing/selftests/bpf/prog_tests/mmap.c+12−1

@@ -19,7 +19,7 @@ void test_mmap(void) · 18 unmodified lines
19	19	const size_t map_sz = roundup_page(sizeof(struct map_data));
20	20	const int zero = 0, one = 1, two = 2, far = 1500;
21	21	const long page_size = sysconf(_SC_PAGE_SIZE);
22		− int err, duration = 0, i, data_map_fd, data_map_id, tmp_fd;
	22	+ int err, duration = 0, i, data_map_fd, data_map_id, tmp_fd, rdmap_fd;
23	23	struct bpf_map data_map, bss_map;
24	24	void bss_mmaped = NULL, map_mmaped = NULL, tmp1, tmp2;
25	25	struct test_mmap__bss *bss_data;
@@ -37,6 +37,17 @@ void test_mmap(void) · 11 unmodified lines
37	37	data_map = skel->maps.data_map;
38	38	data_map_fd = bpf_map__fd(data_map);
39	39
	40	+ rdmap_fd = bpf_map__fd(skel->maps.rdonly_map);
	41	+ tmp1 = mmap(NULL, 4096, PROT_READ \| PROT_WRITE, MAP_SHARED, rdmap_fd, 0);
	42	+ if (CHECK(tmp1 != MAP_FAILED, "rdonly_write_mmap", "unexpected success\n")) {
	43	+ munmap(tmp1, 4096);
	44	+ goto cleanup;
	45	+ }
	46	+ /* now double-check if it's mmap()'able at all */
	47	+ tmp1 = mmap(NULL, 4096, PROT_READ, MAP_SHARED, rdmap_fd, 0);
	48	+ if (CHECK(tmp1 == MAP_FAILED, "rdonly_read_mmap", "failed: %d\n", errno))
	49	+ goto cleanup;
	50	+
40	51	/* get map's ID */
41	52	memset(&map_info, 0, map_info_sz);
42	53	err = bpf_obj_get_info_by_fd(data_map_fd, &map_info, &map_info_sz);

tools/testing/selftests/bpf/progs/test_mmap.c+8−0

@@ -7,6 +7,14 @@ · 6 unmodified lines
7	7
8	8	char _license[] SEC("license") = "GPL";
9	9
	10	+struct {
	11	+ __uint(type, BPF_MAP_TYPE_ARRAY);
	12	+ __uint(max_entries, 4096);
	13	+ __uint(map_flags, BPF_F_MMAPABLE \| BPF_F_RDONLY_PROG);
	14	+ __type(key, __u32);
	15	+ __type(value, char);
	16	+} rdonly_map SEC(".maps");
	17	+
10	18	struct {
11	19	__uint(type, BPF_MAP_TYPE_ARRAY);
12	20	__uint(max_entries, 512 * 4); /* at least 4 pages of data */