On 11/12/2015 04:55 PM, Andrey Ryabinin wrote: > 2015-11-12 18:17 GMT+03:00 Jerome Marchand <jmarchan@xxxxxxxxxx>: >> Commit 71394fe50146 ("mm: vmalloc: add flag preventing guard hole >> allocation") missed a spot. Currently remove_vm_area() decreases >> vm->size to remove the guard hole page, even when it isn't present. >> This patch only decreases vm->size when VM_NO_GUARD isn't set. >> >> Signed-off-by: Jerome Marchand <jmarchan@xxxxxxxxxx> >> --- >> mm/vmalloc.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/mm/vmalloc.c b/mm/vmalloc.c >> index d045634..1388c3d 100644 >> --- a/mm/vmalloc.c >> +++ b/mm/vmalloc.c >> @@ -1443,7 +1443,8 @@ struct vm_struct *remove_vm_area(const void *addr) >> vmap_debug_free_range(va->va_start, va->va_end); >> kasan_free_shadow(vm); >> free_unmap_vmap_area(va); >> - vm->size -= PAGE_SIZE; >> + if (!(vm->flags & VM_NO_GUARD)) >> + vm->size -= PAGE_SIZE; >> > > I'd fix this in another way. I think that remove_vm_area() shouldn't > change vm's size, IMO it doesn't make sense. > The only caller who cares about vm's size after removing is __vunmap(): > area = remove_vm_area(addr); > .... > debug_check_no_locks_freed(addr, area->size); > debug_check_no_obj_freed(addr, area->size); > > We already have proper get_vm_area_size() helper which takes > VM_NO_GUARD into account. > So I think we should use that helper for debug_check_no_*() and just > remove 'vm->size -= PAGE_SIZE;' line > from remove_vm_area() Sure, that would be cleaner. Btw, there might be a leak in sq_unmap() (arch/sh/kernel/cpu/sh4/sq.c) as the vm_struct doesn't seem to be freed. CCed the SuperH folks. Thanks, Jerome > > > >> return vm; >> } >> -- >> 2.4.3 >>
Attachment:
signature.asc
Description: OpenPGP digital signature