History

bpf: Prevent re-mmap()'ing BPF map as writable for initially r/o mapping

VM_MAYWRITE flag during initial memory mapping determines if already mmap()'ed pages can be later remapped as writable ones through mprotect() call. To prevent user application to rewrite contents of memory-mapped as read-only and subsequently frozen BPF map, remove VM_MAYWRITE flag completely on initially read-only mapping. Alternatively, we could treat any memory-mapping on unfrozen map as writable and bump writecnt instead. But there is little legitimate reason to map BPF map as read-only and then re-mmap() it as writable through mprotect(), instead of just mmap()'ing it as read/write from the very beginning. Also, at the suggestion of Jann Horn, drop unnecessary refcounting in mmap operations. We can just rely on VMA holding reference to BPF map's file properly.

Andrii Nakryiko committed 6 years ago

Commit 1f6cb191 parent 41784171 file · +7 −9Browse files

Showing the changes to kernel (1 of 1 file). Show all

Unified · Split

kernel/bpf/syscall.c+7−9
@@ -586,9 +586,7 @@ static void bpf_map_mmap_open(struct vm_area_struct *vma) · 585 unmodified lines
586{586{
587 struct bpf_map *map = vma->vm_file->private_data;587 struct bpf_map *map = vma->vm_file->private_data;
588 588
589 bpf_map_inc_with_uref(map);589 if (vma->vm_flags & VM_MAYWRITE) {
590
591 if (vma->vm_flags & VM_WRITE) {
592 mutex_lock(&map->freeze_mutex);590 mutex_lock(&map->freeze_mutex);
593 map->writecnt++;591 map->writecnt++;
594 mutex_unlock(&map->freeze_mutex);592 mutex_unlock(&map->freeze_mutex);
@@ -600,13 +598,11 @@ static void bpf_map_mmap_close(struct vm_area_struct *vma) · 5 unmodified lines
600{598{
601 struct bpf_map *map = vma->vm_file->private_data;599 struct bpf_map *map = vma->vm_file->private_data;
602 600
603 if (vma->vm_flags & VM_WRITE) {601 if (vma->vm_flags & VM_MAYWRITE) {
604 mutex_lock(&map->freeze_mutex);602 mutex_lock(&map->freeze_mutex);
605 map->writecnt--;603 map->writecnt--;
606 mutex_unlock(&map->freeze_mutex);604 mutex_unlock(&map->freeze_mutex);
607 }605 }
608
609 bpf_map_put_with_uref(map);
610}606}
611 607
612static const struct vm_operations_struct bpf_map_default_vmops = {608static const struct vm_operations_struct bpf_map_default_vmops = {
@@ -635,14 +631,16 @@ static int bpf_map_mmap(struct file *filp, struct vm_area_struct *vma) · 22 unmodified lines
635 /* set default open/close callbacks */631 /* set default open/close callbacks */
636 vma->vm_ops = &bpf_map_default_vmops;632 vma->vm_ops = &bpf_map_default_vmops;
637 vma->vm_private_data = map;633 vma->vm_private_data = map;
634 vma->vm_flags &= ~VM_MAYEXEC;
635 if (!(vma->vm_flags & VM_WRITE))
636 /* disallow re-mapping with PROT_WRITE */
637 vma->vm_flags &= ~VM_MAYWRITE;
638 638
639 err = map->ops->map_mmap(map, vma);639 err = map->ops->map_mmap(map, vma);
640 if (err)640 if (err)
641 goto out;641 goto out;
642 642
643 bpf_map_inc_with_uref(map);643 if (vma->vm_flags & VM_MAYWRITE)
644
645 if (vma->vm_flags & VM_WRITE)
646 map->writecnt++;644 map->writecnt++;
647out:645out:
648 mutex_unlock(&map->freeze_mutex);646 mutex_unlock(&map->freeze_mutex);