Message ID | 20240712-asi-rfc-24-v1-18-144b319a40d8@google.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Address Space Isolation (ASI) 2024 | expand |
Hi Brendan, kernel test robot noticed the following build warnings: [auto build test WARNING on a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6] url: https://github.com/intel-lab-lkp/linux/commits/Brendan-Jackman/mm-asi-Make-some-utility-functions-noinstr-compatible/20240713-012107 base: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6 patch link: https://lore.kernel.org/r/20240712-asi-rfc-24-v1-18-144b319a40d8%40google.com patch subject: [PATCH 18/26] mm: asi: Map vmalloc/vmap data as nonsesnitive config: openrisc-allnoconfig (https://download.01.org/0day-ci/archive/20240713/202407132325.YotidwGR-lkp@intel.com/config) compiler: or1k-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240713/202407132325.YotidwGR-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202407132325.YotidwGR-lkp@intel.com/ All warnings (new ones prefixed by >>): mm/vmalloc.c: In function 'remove_vm_area': >> mm/vmalloc.c:3192:23: warning: variable 'vm_addr' set but not used [-Wunused-but-set-variable] 3192 | unsigned long vm_addr; | ^~~~~~~ vim +/vm_addr +3192 mm/vmalloc.c 3177 3178 /** 3179 * remove_vm_area - find and remove a continuous kernel virtual area 3180 * @addr: base address 3181 * 3182 * Search for the kernel VM area starting at @addr, and remove it. 3183 * This function returns the found VM area, but using it is NOT safe 3184 * on SMP machines, except for its size or flags. 3185 * 3186 * Return: the area descriptor on success or %NULL on failure. 3187 */ 3188 struct vm_struct *remove_vm_area(const void *addr) 3189 { 3190 struct vmap_area *va; 3191 struct vm_struct *vm; > 3192 unsigned long vm_addr; 3193 3194 might_sleep(); 3195 3196 if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n", 3197 addr)) 3198 return NULL; 3199 3200 va = find_unlink_vmap_area((unsigned long)addr); 3201 if (!va || !va->vm) 3202 return NULL; 3203 vm = va->vm; 3204 vm_addr = (unsigned long) READ_ONCE(vm->addr); 3205 3206 debug_check_no_locks_freed(vm->addr, get_vm_area_size(vm)); 3207 debug_check_no_obj_freed(vm->addr, get_vm_area_size(vm)); 3208 kasan_free_module_shadow(vm); 3209 kasan_poison_vmalloc(vm->addr, get_vm_area_size(vm)); 3210 3211 free_unmap_vmap_area(va); 3212 return vm; 3213 } 3214
diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 7a8daf5afb7c..d14e2f692e42 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -3189,6 +3189,7 @@ struct vm_struct *remove_vm_area(const void *addr) { struct vmap_area *va; struct vm_struct *vm; + unsigned long vm_addr; might_sleep(); @@ -3200,6 +3201,7 @@ struct vm_struct *remove_vm_area(const void *addr) if (!va || !va->vm) return NULL; vm = va->vm; + vm_addr = (unsigned long) READ_ONCE(vm->addr); debug_check_no_locks_freed(vm->addr, get_vm_area_size(vm)); debug_check_no_obj_freed(vm->addr, get_vm_area_size(vm)); @@ -3331,6 +3333,7 @@ void vfree(const void *addr) addr); return; } + asi_unmap(ASI_GLOBAL_NONSENSITIVE, vm->addr, get_vm_area_size(vm)); if (unlikely(vm->flags & VM_FLUSH_RESET_PERMS)) vm_reset_perms(vm); @@ -3370,12 +3373,14 @@ void vunmap(const void *addr) if (!addr) return; + vm = remove_vm_area(addr); if (unlikely(!vm)) { WARN(1, KERN_ERR "Trying to vunmap() nonexistent vm area (%p)\n", addr); return; } + asi_unmap(ASI_GLOBAL_NONSENSITIVE, vm->addr, get_vm_area_size(vm)); kfree(vm); } EXPORT_SYMBOL(vunmap); @@ -3424,16 +3429,21 @@ void *vmap(struct page **pages, unsigned int count, addr = (unsigned long)area->addr; if (vmap_pages_range(addr, addr + size, pgprot_nx(prot), - pages, PAGE_SHIFT) < 0) { - vunmap(area->addr); - return NULL; - } + pages, PAGE_SHIFT) < 0) + goto err; + + if (asi_map(ASI_GLOBAL_NONSENSITIVE, area->addr, + get_vm_area_size(area))) + goto err; /* The necessary asi_unmap() is in vunmap. */ if (flags & VM_MAP_PUT_PAGES) { area->pages = pages; area->nr_pages = count; } return area->addr; +err: + vunmap(area->addr); + return NULL; } EXPORT_SYMBOL(vmap); @@ -3701,6 +3711,10 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask, goto fail; } + if (asi_map(ASI_GLOBAL_NONSENSITIVE, area->addr, + get_vm_area_size(area))) + goto fail; /* The necessary asi_unmap() is in vfree. */ + return area->addr; fail: @@ -3780,6 +3794,13 @@ void *__vmalloc_node_range(unsigned long size, unsigned long align, size = ALIGN(real_size, 1UL << shift); } + /* + * Assume nobody is interested in accessing these pages via the direct + * map, so there's no point in having them in ASI's global-nonsensitive + * physmap, which would just cost us a TLB flush later on. + */ + gfp_mask |= __GFP_SENSITIVE; + again: area = __get_vm_area_node(real_size, align, shift, VM_ALLOC | VM_UNINITIALIZED | vm_flags, start, end, node,
We add new VM flags for sensitive and global-nonsensitive, parallel to the corresponding GFP flags. __get_vm_area_node and friends will default to creating global-nonsensitive VM areas, and vmap then calls asi_map as necessary. __vmalloc_node_range has additional logic to check and set defaults for the sensitivity of the underlying page allocation. It does this via an initial __set_asi_flags call - note that it then calls __get_vm_area_node which also calls __set_asi_flags. This second call is a NOP. By default, we mark the underlying page allocation as sensitive, even if the VM area is global-nonsensitive. This is just an optimization to avoid unnecessary asi_map etc, since presumably most code has no reason to access vmalloc'd data through the direct map. There are some details of the GFP-flag/VM-flag interaction that are not really obvious, for example: what should happen when callers of __vmalloc explicitly set GFP sensitivity flags? (That function has no VM flags argument). For the moment let's just not block on that and focus on adding the infastructure, though. At the moment, the high-level vmalloc APIs doesn't actually provide a way to conffigure sensitivity, this commit just adds the infrastructure. We'll have to decide how to expose this to allocation sites as we implement more denylist logic. vmap does already allow configuring vm flags. Signed-off-by: Brendan Jackman <jackmanb@google.com> --- mm/vmalloc.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)