Skip to content

Commit bdebd6a

Browse files
thejhtorvalds
authored andcommitted
vmalloc: fix remap_vmalloc_range() bounds checks
remap_vmalloc_range() has had various issues with the bounds checks it promises to perform ("This function checks that addr is a valid vmalloc'ed area, and that it is big enough to cover the vma") over time, e.g.: - not detecting pgoff<<PAGE_SHIFT overflow - not detecting (pgoff<<PAGE_SHIFT)+usize overflow - not checking whether addr and addr+(pgoff<<PAGE_SHIFT) are the same vmalloc allocation - comparing a potentially wildly out-of-bounds pointer with the end of the vmalloc region In particular, since commit fc97022 ("bpf: Add mmap() support for BPF_MAP_TYPE_ARRAY"), unprivileged users can cause kernel null pointer dereferences by calling mmap() on a BPF map with a size that is bigger than the distance from the start of the BPF map to the end of the address space. This could theoretically be used as a kernel ASLR bypass, by using whether mmap() with a given offset oopses or returns an error code to perform a binary search over the possible address range. To allow remap_vmalloc_range_partial() to verify that addr and addr+(pgoff<<PAGE_SHIFT) are in the same vmalloc region, pass the offset to remap_vmalloc_range_partial() instead of adding it to the pointer in remap_vmalloc_range(). In remap_vmalloc_range_partial(), fix the check against get_vm_area_size() by using size comparisons instead of pointer comparisons, and add checks for pgoff. Fixes: 8334231 ("[PATCH] mm: introduce remap_vmalloc_range()") Signed-off-by: Jann Horn <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Cc: [email protected] Cc: Alexei Starovoitov <[email protected]> Cc: Daniel Borkmann <[email protected]> Cc: Martin KaFai Lau <[email protected]> Cc: Song Liu <[email protected]> Cc: Yonghong Song <[email protected]> Cc: Andrii Nakryiko <[email protected]> Cc: John Fastabend <[email protected]> Cc: KP Singh <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Linus Torvalds <[email protected]>
1 parent 0783ac9 commit bdebd6a

File tree

4 files changed

+18
-7
lines changed

4 files changed

+18
-7
lines changed

fs/proc/vmcore.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,8 @@ static int vmcoredd_mmap_dumps(struct vm_area_struct *vma, unsigned long dst,
266266
if (start < offset + dump->size) {
267267
tsz = min(offset + (u64)dump->size - start, (u64)size);
268268
buf = dump->buf + start - offset;
269-
if (remap_vmalloc_range_partial(vma, dst, buf, tsz)) {
269+
if (remap_vmalloc_range_partial(vma, dst, buf, 0,
270+
tsz)) {
270271
ret = -EFAULT;
271272
goto out_unlock;
272273
}
@@ -624,7 +625,7 @@ static int mmap_vmcore(struct file *file, struct vm_area_struct *vma)
624625
tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)start, size);
625626
kaddr = elfnotes_buf + start - elfcorebuf_sz - vmcoredd_orig_sz;
626627
if (remap_vmalloc_range_partial(vma, vma->vm_start + len,
627-
kaddr, tsz))
628+
kaddr, 0, tsz))
628629
goto fail;
629630

630631
size -= tsz;

include/linux/vmalloc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ extern void vunmap(const void *addr);
137137

138138
extern int remap_vmalloc_range_partial(struct vm_area_struct *vma,
139139
unsigned long uaddr, void *kaddr,
140-
unsigned long size);
140+
unsigned long pgoff, unsigned long size);
141141

142142
extern int remap_vmalloc_range(struct vm_area_struct *vma, void *addr,
143143
unsigned long pgoff);

mm/vmalloc.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <linux/llist.h>
3535
#include <linux/bitops.h>
3636
#include <linux/rbtree_augmented.h>
37+
#include <linux/overflow.h>
3738

3839
#include <linux/uaccess.h>
3940
#include <asm/tlbflush.h>
@@ -3054,6 +3055,7 @@ long vwrite(char *buf, char *addr, unsigned long count)
30543055
* @vma: vma to cover
30553056
* @uaddr: target user address to start at
30563057
* @kaddr: virtual address of vmalloc kernel memory
3058+
* @pgoff: offset from @kaddr to start at
30573059
* @size: size of map area
30583060
*
30593061
* Returns: 0 for success, -Exxx on failure
@@ -3066,9 +3068,15 @@ long vwrite(char *buf, char *addr, unsigned long count)
30663068
* Similar to remap_pfn_range() (see mm/memory.c)
30673069
*/
30683070
int remap_vmalloc_range_partial(struct vm_area_struct *vma, unsigned long uaddr,
3069-
void *kaddr, unsigned long size)
3071+
void *kaddr, unsigned long pgoff,
3072+
unsigned long size)
30703073
{
30713074
struct vm_struct *area;
3075+
unsigned long off;
3076+
unsigned long end_index;
3077+
3078+
if (check_shl_overflow(pgoff, PAGE_SHIFT, &off))
3079+
return -EINVAL;
30723080

30733081
size = PAGE_ALIGN(size);
30743082

@@ -3082,8 +3090,10 @@ int remap_vmalloc_range_partial(struct vm_area_struct *vma, unsigned long uaddr,
30823090
if (!(area->flags & (VM_USERMAP | VM_DMA_COHERENT)))
30833091
return -EINVAL;
30843092

3085-
if (kaddr + size > area->addr + get_vm_area_size(area))
3093+
if (check_add_overflow(size, off, &end_index) ||
3094+
end_index > get_vm_area_size(area))
30863095
return -EINVAL;
3096+
kaddr += off;
30873097

30883098
do {
30893099
struct page *page = vmalloc_to_page(kaddr);
@@ -3122,7 +3132,7 @@ int remap_vmalloc_range(struct vm_area_struct *vma, void *addr,
31223132
unsigned long pgoff)
31233133
{
31243134
return remap_vmalloc_range_partial(vma, vma->vm_start,
3125-
addr + (pgoff << PAGE_SHIFT),
3135+
addr, pgoff,
31263136
vma->vm_end - vma->vm_start);
31273137
}
31283138
EXPORT_SYMBOL(remap_vmalloc_range);

samples/vfio-mdev/mdpy.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ static int mdpy_mmap(struct mdev_device *mdev, struct vm_area_struct *vma)
418418
return -EINVAL;
419419

420420
return remap_vmalloc_range_partial(vma, vma->vm_start,
421-
mdev_state->memblk,
421+
mdev_state->memblk, 0,
422422
vma->vm_end - vma->vm_start);
423423
}
424424

0 commit comments

Comments
 (0)