mm: vmalloc: Support more granular vrealloc() sizing
Subsystems (2)
PATCH 1/1 mm: vmalloc: Support more granular vrealloc() sizing
Introduce struct vm_struct::requested_size so that the requested (re)allocation size is retained separately from the allocated area size. This means that KASAN will correctly poison the correct spans of requested bytes. This also means we can support growing the usable portion of an allocation that can already be supported by the existing area's existing allocation.
---
Cc: Andrew Morton <[email protected]>
Cc: Danilo Krummrich <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: Uladzislau Rezki <[email protected]>
Cc: <[email protected]>
--- include/linux/vmalloc.h | 1 + mm/vmalloc.c | 30 ++++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-)
include/linux/vmalloc.h +1 -0
@@ -61,6 +61,7 @@ struct vm_struct {unsigned int nr_pages;phys_addr_t phys_addr;const void *caller; +unsigned long requested_size; };struct vmap_area {
mm/vmalloc.c +24 -6
@@ -1940,7 +1940,7 @@ static inline void setup_vmalloc_vm(struct vm_struct *vm, { vm->flags = flags; vm->addr = (void *)va->va_start; - vm->size =va_size (va); + vm->size = vm->requested_size =va_size (va); vm->caller = caller; va->vm = vm; }
@@ -3133,6 +3133,7 @@ struct vm_struct *__get_vm_area_node(unsigned long size, area->flags = flags; area->caller = caller; + area->requested_size = requested_size; va =alloc_vmap_area (size, align, start, end, node, gfp_mask,0 , area);if (IS_ERR (va)) {
@@ -4063,6 +4064,8 @@ EXPORT_SYMBOL(vzalloc_node_noprof);*/ void *vrealloc_noprof (const void *p,size_t size,gfp_t flags) { +struct vm_struct *vm =NULL ; +size_t alloced_size =0 ;size_t old_size =0 ;void *n;
@@ -4072,15 +4075,17 @@ void *vrealloc_noprof(const void *p, size_t size, gfp_t flags) }if (p) { -struct vm_struct *vm; - vm =find_vm_area (p);if (unlikely (!vm)) {WARN (1 ,"Trying to vrealloc() nonexistent vm area (%p)\n" , p);return NULL ; } - old_size =get_vm_area_size (vm); + alloced_size =get_vm_area_size (vm); + old_size = vm->requested_size; +if (WARN (alloced_size < old_size, +"vrealloc() has mismatched area vs requested sizes (%p)\n" , p)) +return NULL ; }/*
@@ -4088,14 +4093,27 @@ void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)* would be a good heuristic for when to shrink the vm_area? */ if (size <= old_size) { -/* Zero out spare memory. */ -if (want_init_on_alloc (flags)) +/* Zero out "freed" memory. */ +if (want_init_on_free ())memset ((void *)p + size,0 , old_size - size); + vm->requested_size = size;kasan_poison_vmalloc (p + size, old_size - size);kasan_unpoison_vmalloc (p, size,KASAN_VMALLOC_PROT_NORMAL );return (void *)p; } +/* +* We already have the bytes available in the allocation; use them. +*/ +if (size <= alloced_size) { +kasan_unpoison_vmalloc (p, size,KASAN_VMALLOC_PROT_NORMAL ); +/* Zero out "alloced" memory. */ +if (want_init_on_alloc (flags)) +memset ((void *)p + old_size,0 , size - old_size); + vm->requested_size = size; +kasan_poison_vmalloc (p + size, alloced_size - size); + } +/* TODO: Grow the vm_area, i.e. allocate and map additional pages. */ n =__vmalloc_noprof (size, flags);if (!n)
Signature · 1 line
2.34.1
Quoted 11 lines · Kees Cook
Introduce struct
vm_struct::requested_sizeso that the requested (re)allocation size is retained separately from the allocated area size. This means that KASAN will correctly poison the correct spans of requested bytes. This also means we can support growing the usable portion of an allocation that can already be supported by the existing area's existing allocation.Reported-by Erhard Furtner <[email protected]>Fixes 3ddc2fefe6f3 ("mm: vmalloc: implement vrealloc()")Signed-off-by Kees Cook <[email protected]>
Good catch!
One question below, otherwise
Quoted 15 lines
mm/vmalloc.c +3
-2@@ -4088,14 +4093,27 @@ void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)* would be a good heuristic for when to shrink the vm_area? */ if (size <= old_size) { -/* Zero out spare memory. */ -if (want_init_on_alloc (flags)) +/* Zero out "freed" memory. */ +if (want_init_on_free ())memset ((void *)p + size,0 , old_size - size); + vm->requested_size = size;kasan_poison_vmalloc (p + size, old_size - size);kasan_unpoison_vmalloc (p, size,KASAN_VMALLOC_PROT_NORMAL );return (void *)p; }
mm/vmalloc.c void *vrealloc_noprof(const void *p, size_t size, gfp_t flags) · line 4114
Quoted 10 lines
mm/vmalloc.c +10
-0+/* +* We already have the bytes available in the allocation; use them. +*/ +if (size <= alloced_size) { +kasan_unpoison_vmalloc (p, size,KASAN_VMALLOC_PROT_NORMAL ); +/* Zero out "alloced" memory. */ +if (want_init_on_alloc (flags)) +memset ((void *)p + old_size,0 , size - old_size); + vm->requested_size = size; +kasan_poison_vmalloc (p + size, alloced_size - size);
Do we need this? We know that old_size < size <= alloced_size. And since previously [p + old_size, p + alloced_size) must have been poisoned, [p + size, p + alloced_size) must be poisoned already?
Maybe there was a reason, since in the above (size <= old_size) case kasan_unpoison_vmalloc() seems unnecessary too.
Quoted 49 lines · Danilo Krummrich
Quoted 11 lines · Kees Cook
Introduce struct
vm_struct::requested_sizeso that the requested (re)allocation size is retained separately from the allocated area size. This means that KASAN will correctly poison the correct spans of requested bytes. This also means we can support growing the usable portion of an allocation that can already be supported by the existing area's existing allocation.Reported-by Erhard Furtner <[email protected]>Fixes 3ddc2fefe6f3 ("mm: vmalloc: implement vrealloc()")Signed-off-by Kees Cook <[email protected]>Good catch!
One question below, otherwise
Reviewed-by Danilo Krummrich <[email protected]>Quoted 15 lines
mm/vmalloc.c +3
-2@@ -4088,14 +4093,27 @@ void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)* would be a good heuristic for when to shrink the vm_area? */ if (size <= old_size) { -/* Zero out spare memory. */ -if (want_init_on_alloc (flags)) +/* Zero out "freed" memory. */ +if (want_init_on_free ())memset ((void *)p + size,0 , old_size - size); + vm->requested_size = size;kasan_poison_vmalloc (p + size, old_size - size);kasan_unpoison_vmalloc (p, size,KASAN_VMALLOC_PROT_NORMAL );return (void *)p; }mm/vmalloc.c void *vrealloc_noprof(const void *p, size_t size, gfp_t flags) · line 4114
Quoted 10 lines
mm/vmalloc.c +10
-0+/* +* We already have the bytes available in the allocation; use them. +*/ +if (size <= alloced_size) { +kasan_unpoison_vmalloc (p, size,KASAN_VMALLOC_PROT_NORMAL ); +/* Zero out "alloced" memory. */ +if (want_init_on_alloc (flags)) +memset ((void *)p + old_size,0 , size - old_size); + vm->requested_size = size; +kasan_poison_vmalloc (p + size, alloced_size - size);Do we need this? We know that
old_size< size <=alloced_size. And since previously [p +old_size, p +alloced_size) must have been poisoned, [p + size, p +alloced_size) must be poisoned already?
Quoted 2 lines
Maybe there was a reason, since in the above (size <=
old_size) casekasan_unpoison_vmalloc()seems unnecessary too.
Honestly I was just copying the logic from the prior case. But yeah, it should be possible (in both cases) to just apply the changed span. For the "size <= old_size" case, it would just be:
kasan_poison_vmalloc(p + size, old_size - size);
(i.e. the kasan_unpoison_vmalloc() call isn't needed at all, as you say.)
And in the "size <= alloced_size" case, it would just be:
kasan_unpoison_vmalloc(p + old_size, size - old_size, KASAN_VMALLOC_PROT_NORMAL);
and no kasan_poison_vmalloc() should be needed.
Do the KASAN folks on CC have any opinion on best practices here?
Thanks for looking it over!
-Kees
Signature · 1 line
Kees Cook