Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations
Kees Cook <[email protected]> Wed, 22 Jul 2026 21:03:49 -0700
| Newsgroups | org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <202607222102.E6A5679@keescook> |
On Wed, Jul 22, 2026 at 02:29:35PM +0000, Dev Jain wrote: > The vmalloc allocator stores the actual allocation size inside the > vm_struct structure. We can use this bound in usercopy instead of the > page-aligned va_end to catch usercopy beyond the actual allocation size. > > For vmap, the requested_size field is always page-aligned since it maps a > certain number of pages. Same for vm_map_ram (alongwith, not even having > a vm_struct). So the check is only relevant for vmalloc mappings. > > Because there are early vm areas registered even before vmalloc_init, > requested_size may be zero. So also check whether the requested_size > is set. > > Signed-off-by: Dev Jain <[email protected]> Looks good to me! Can you update the LKDTM tests to check for this more tightened range check with a new vmalloc-based test? If other mm folks can double-check this, I'll happily take this via the hardening tree. Thanks! -Kees > --- > Applies on mm-new (3d18f3499c48). > > mm/usercopy.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/mm/usercopy.c b/mm/usercopy.c > index 5de7a518b1b1c..772681aff8477 100644 > --- a/mm/usercopy.c > +++ b/mm/usercopy.c > @@ -176,10 +176,28 @@ static inline void check_heap_object(const void *ptr, unsigned long n, > > if (is_vmalloc_addr(ptr) && !pagefault_disabled()) { > struct vmap_area *area = find_vmap_area(addr); > + struct vm_struct *vm; > > if (!area) > usercopy_abort("vmalloc", "no area", to_user, 0, n); > > + vm = area->vm; > + /* > + * Mappings with a vm_struct track the originally requested > + * size. Check against that rather than the page-rounded > + * vmap_area->va_end so copies cannot reach vmalloc tail > + * padding. vmap mappings are always page aligned. > + */ > + if (vm && (vm->flags & VM_ALLOC) && vm->requested_size) { > + unsigned long size = vm->requested_size; > + > + offset = addr - area->va_start; > + if (offset > size || n > size - offset) > + usercopy_abort("vmalloc", NULL, to_user, > + offset, n); > + return; > + } > + > if (n > area->va_end - addr) { > offset = addr - area->va_start; > usercopy_abort("vmalloc", NULL, to_user, offset, n); > -- > 2.43.0 > -- Kees Cook