Re: [PATCH 2/3] mm/usercopy: Detect vmalloc overruns

[Kees Cook <keescook@chromium.org>]

On Mon, Oct 04, 2021 at 11:42:22PM +0100, Matthew Wilcox (Oracle) wrote:
> If you have a vmalloc() allocation, or an address from calling vmap(),
> you cannot overrun the vm_area which describes it, regardless of the
> size of the underlying allocation.  This probably doesn't do much for
> security because vmalloc comes with guard pages these days, but it
> prevents usercopy aborts when copying to a vmap() of smaller pages.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/usercopy.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index ac95b22fbbce..7bfc4f9ed1e4 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -17,6 +17,7 @@
>  #include <linux/sched/task.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/thread_info.h>
> +#include <linux/vmalloc.h>
>  #include <linux/atomic.h>
>  #include <linux/jump_label.h>
>  #include <asm/sections.h>
> @@ -236,6 +237,14 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
>  		return;
>  	}
>  
> +	if (is_vmalloc_addr(ptr)) {
> +		struct vm_struct *vm = find_vm_area(ptr);
> +
> +		if (ptr + n > vm->addr + vm->size)
> +			usercopy_abort("vmalloc", NULL, to_user, 0, n);

This "0" is easy to make "ptr - vm->addr". With that fixed:

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> +		return;
> +	}
> +
>  	page = virt_to_head_page(ptr);
>  
>  	if (PageSlab(page)) {
> -- 
> 2.32.0
> 

-- 
Kees Cook