Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations
Uladzislau Rezki <[email protected]> Mon, 27 Jul 2026 11:26:41 +0200
| Newsgroups | org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <amckUdXO-7TtzhNg@milan> |
On Mon, Jul 27, 2026 at 12:33:49PM +0530, Dev Jain wrote: > > > On 26/07/26 4:16 pm, Matthew Wilcox wrote: > > On Sun, Jul 26, 2026 at 02:23:20PM +0530, Dev Jain wrote: > >> On 22/07/26 7:59 pm, 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]> > >>> --- > >> > >> Sashiko: > >> > >> 1. "Does this locklessly access area->vm after find_vmap_area() has dropped > >> the busy tree lock? > >> > >> If an out-of-bounds pointer falls into an adjacent vmap_area, and that > >> adjacent area is concurrently freed by another thread, its vm_struct > >> is freed. Additionally, when the vmap_area is moved to the free tree, > >> area->vm (which shares a union with subtree_max_size) is overwritten > >> with an integer size. > >> > >> Would dereferencing vm->flags later in this function cause a use-after-free > >> or a wild pointer dereference?" > >> > >> > >> I don't get it. So usercopy is checking OOB for an object but shouldn't > >> assume the existence of that object while using it? > >> > >> It is a bug in the caller if someone does vfree() while usercopy is operating > >> on the vmalloc object. I don't think usercopy should handle it. For example > >> we don't handle it for slabs currently. > > > > I think the question Sashiko is getting to is how we handle: > > > > char *p = vmalloc(); > > copy_to_user(p - 4); > > Thanks Willy, I completely misread Sashiko's point. > > I think this is an existing problem? We are using area->va_end currently, > so we can access freed memory. > It is a bit mess here since now we need also take into account a requested_size due to vrealloc. We have requested_size, nr_pages and size in the vm_struct. -- Uladzislau Rezki