[PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[Dev Jain]

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 <dev.jain@arm.com>
---
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

Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[Kees Cook]

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 <dev.jain@arm.com>

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

Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[Dev Jain]

On 23/07/26 9:33 am, Kees Cook wrote:
> 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 <dev.jain@arm.com>
> 
> Looks good to me! Can you update the LKDTM tests to check for this more
> tightened range check with a new vmalloc-based test?

Thanks. I have written the test, I'll send a v2 with it once the mm people
take a look at this.


> 
> 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
>>
> 

Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[Barry Song]

On Wed, Jul 22, 2026 at 10:43 PM Dev Jain <dev.jain@arm.com> 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 <dev.jain@arm.com>
> ---

I wrote the test module below to verify the impact on user-space behavior.

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/proc_fs.h>
#include <linux/uaccess.h>
#include <linux/vmalloc.h>

#define PROC_NAME "copy_bug"

static ssize_t copy_bug_read(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
        volatile int alloc_len = 5 * 1024, copy_len = 6 * 1024;
        void *kbuf;
        int ret;

        if (*ppos)
                return 0;

        kbuf = vmalloc(alloc_len);
        if (!kbuf)
                return -ENOMEM;
        memset(kbuf, 0, alloc_len);

        ret = copy_to_user(buf, kbuf, copy_len);
        pr_info("copy_bug: copy_to_user returned %d count:%ld\n", ret, count);
        vfree(kbuf);
        return count;
}

static const struct proc_ops copy_bug_ops = {
        .proc_read = copy_bug_read,
};

static int __init copy_bug_init(void)
{
        proc_create(PROC_NAME, 0444, NULL, &copy_bug_ops);
        return 0;
}

static void __exit copy_bug_exit(void)
{
        remove_proc_entry(PROC_NAME, NULL);
}

module_init(copy_bug_init);
module_exit(copy_bug_exit);
MODULE_LICENSE("GPL");

without the patch:
/ # cat /proc/copy_bug
[    6.517711] copy_bug: copy_to_user returned 0 count:131072
[    6.655611] copy_bug: copy_to_user returned 0 count:131072
[    6.783372] copy_bug: copy_to_user returned 0 count:131072
[    6.912167] copy_bug: copy_to_user returned 0 count:131072
[    7.039019] copy_bug: copy_to_user returned 0 count:131072

with the patch:

/ # cat /proc/copy_bug
[   11.298890] usercopy: Kernel memory exposure attempt detected from
vmalloc (offset 0, size 6144)!
[   11.299246] ------------[ cut here ]------------
[   11.299276] kernel BUG at mm/usercopy.c:102!
[   11.299616] Internal error: Oops - BUG: 00000000f2000800 [#1]  SMP
[   11.300458] CPU: 1 UID: 0 PID: 74 Comm: cat Not tainted
7.2.0-rc4-g6b5aeb5cf21a-dirty #1033 PREEMPT
[   11.300730] Hardware name: linux,dummy-virt (DT)
[   11.300940] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   11.301088] pc : usercopy_abort+0x94/0x98
[   11.301190] lr : usercopy_abort+0x94/0x98
[   11.301280] sp : ffff800083643c40
[   11.301358] x29: ffff800083643c50 x28: ffff000004373700 x27: 0000000000000000
[   11.301994] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000001
[   11.302152] x23: 0000000000020000 x22: 0000000000000001 x21: ffff8000833a6800
[   11.302298] x20: 0000000000001800 x19: ffff8000833a5000 x18: 00000000ffffffff
[   11.302440] x17: 2820636f6c6c616d x16: 76206d6f72662064 x15: 6574636574656420
[   11.302580] x14: 74706d6574746120 x13: 0000000000020000 x12: 0000000000008000
[   11.302720] x11: ffff800082271138 x10: ffff800082181534 x9 : ffff80008012842c
[   11.302878] x8 : ffff000004373700 x7 : ffff80008012782c x6 : 0000000000000001
[   11.303024] x5 : 0000000000001297 x4 : 0000000000000000 x3 : 0000000000000000
[   11.303162] x2 : 0000000000000000 x1 : ffff000004373700 x0 : 0000000000000055
[   11.303378] Call trace:
[   11.303572]  usercopy_abort+0x94/0x98 (P)
[   11.303744]  __check_object_size+0x1dc/0x360
[   11.303834]  copy_bug_read+0x9c/0x1a0
[   11.303910]  proc_reg_read+0xa4/0x100
[   11.303986]  vfs_read+0xcc/0x2f8
[   11.304064]  ksys_read+0x74/0x118
[   11.304134]  __arm64_sys_read+0x24/0x38
[   11.304212]  invoke_syscall+0x5c/0x120
[   11.304288]  el0_svc_common.constprop.0+0x48/0xf0
[   11.304378]  do_el0_svc+0x24/0x38
[   11.304448]  el0_svc+0x58/0x3f0
[   11.304518]  el0t_64_sync_handler+0xa0/0xe8
[   11.304598]  el0t_64_sync+0x1ac/0x1b0
[   11.304814] Code: aa0003e3 d000e3e0 91378000 97ffec92 (d4210000)
[   11.305138] ---[ end trace 0000000000000000 ]---
[   11.305390] note: cat[74] exited with irqs disabled
[   11.305542] note: cat[74] exited with preempt_count 1
[   11.307308] ------------[ cut here ]------------

So:

Tested-by: Barry Song <baohua@kernel.org>
Reviewed-by: Barry Song <baohua@kernel.org>

Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[Dev Jain]

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 <dev.jain@arm.com>
> ---

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.


2. "Can addr still have hardware KASAN tags (like ARM64 MTE) attached in the
top byte here?

Because check_heap_object() casts ptr to addr without calling
kasan_reset_tag(), a tagged address will be numerically different from
untagged kernel addresses. When calculating offset against the untagged
area->va_start, this could underflow or produce a massive unsigned value.

Could this cause the offset > size check to evaluate to true, triggering a
spurious usercopy_abort() for valid usercopies on HW-tagged vmalloc memory?"


This looks genuine. check_heap_object() should be doing a kasan_reset_tag().
We probably didn't catch this because no one is running CONFIG_HARDENED_USERCOPY
with KASAN enabled, since the latter kind-of already stops the OOB bugs.

Even if they did, mostly we care about slab OOB's I think, and the __check_heap_object
in mm/slub.c does a kasan_reset_tag() already.


> 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);

Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[Dev Jain]

On 26/07/26 2:23 pm, 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 <dev.jain@arm.com>
>> ---
> 
> 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.
> 
> 
> 2. "Can addr still have hardware KASAN tags (like ARM64 MTE) attached in the
> top byte here?
> 
> Because check_heap_object() casts ptr to addr without calling
> kasan_reset_tag(), a tagged address will be numerically different from
> untagged kernel addresses. When calculating offset against the untagged
> area->va_start, this could underflow or produce a massive unsigned value.
> 
> Could this cause the offset > size check to evaluate to true, triggering a
> spurious usercopy_abort() for valid usercopies on HW-tagged vmalloc memory?"
> 
> 
> This looks genuine. check_heap_object() should be doing a kasan_reset_tag().
> We probably didn't catch this because no one is running CONFIG_HARDENED_USERCOPY
> with KASAN enabled, since the latter kind-of already stops the OOB bugs.
> 
> Even if they did, mostly we care about slab OOB's I think, and the __check_heap_object
> in mm/slub.c does a kasan_reset_tag() already.


To be clear, this is an existing bug, so shouldn't stop this patch from going in.

Re: [PATCH] mm/usercopy: harden bounds checking for vmalloc allocations

[Matthew Wilcox]

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 <dev.jain@arm.com>
> > ---
> 
> 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);

My quibble with this patch is that you've made this more precise at the
cost of making it more expensive.  Is the extra precision worth the
cost?  You're not running into anybody else's allocation (ie these pages
are allocated to you exclusively), so I would tend to think not.  It's
like calling kmalloc(93) and then accessing bytes 93-95.  It's out of
bounds, but they're not anybody else's memory.