Re: [linux-next:master] [mm/vmalloc] ff6f2b81ea: WARNING:at_kernel/fork.c:#vm_area_init_from

[Uladzislau Rezki <urezki@gmail.com>]

On Tue, Mar 11, 2025 at 03:26:59PM +0800, liuye wrote:
> 
> 
> 在 2025/3/11 00:52, Uladzislau Rezki 写道:
> > Hello, Andrew, Liu Ye.
> > 
> >>
> >> Hello,
> >>
> >> kernel test robot noticed "WARNING:at_kernel/fork.c:#vm_area_init_from" on:
> >>
> >> commit: ff6f2b81eaa8a9fe5d158c6e7b1e58d3929c32c1 ("mm/vmalloc: move free_vm_area(area) from the __vmalloc_area_node function to the __vmalloc_node_range_noprof function")
> >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
> >>
> >> [test failed on linux-next/master 0a2f889128969dab41861b6e40111aa03dc57014]
> >>
> >> in testcase: trinity
> >> version: 
> >> with following parameters:
> >>
> >> 	runtime: 300s
> >> 	group: group-02
> >> 	nr_groups: 5
> >>
> >>
> >>
> >> config: x86_64-randconfig-101-20250306
> >> compiler: gcc-12
> >> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
> >>
> >> (please refer to attached dmesg/kmsg for entire log/backtrace)
> >>
> >>
> >> +-------------------------------------------------------------+------------+------------+
> >> |                                                             | fb8faf4337 | ff6f2b81ea |
> >> +-------------------------------------------------------------+------------+------------+
> >> | boot_successes                                              | 9          | 0          |
> >> | boot_failures                                               | 0          | 6          |
> >> | WARNING:at_kernel/fork.c:#vm_area_init_from                 | 0          | 6          |
> >> | RIP:vm_area_init_from                                       | 0          | 6          |
> >> | BUG:KASAN:slab-use-after-free_in__vmalloc_node_range_noprof | 0          | 5          |
> >> | WARNING:at_mm/vmalloc.c:#remove_vm_area                     | 0          | 5          |
> >> | RIP:remove_vm_area                                          | 0          | 5          |
> >> | kernel_BUG_at_mm/vmalloc.c                                  | 0          | 5          |
> >> | Oops:invalid_opcode:#[##]PREEMPT_KASAN                      | 0          | 5          |
> >> | RIP:__vmalloc_node_range_noprof                             | 0          | 5          |
> >> | Kernel_panic-not_syncing:Fatal_exception                    | 0          | 5          |
> >> +-------------------------------------------------------------+------------+------------+
> >>
> > The patch that is in question, indeed, looks buggy. At least i can see
> > how a use-after-free can occur:
> > 
> > <snip>
> > static void *__vmalloc_area_node(...)
> > ...
> > fail:
> > 	vfree(area->addr);
> > 	return NULL;
> > }
> > <snip>
> > 
> > <snip>
> > ...
> > ret = __vmalloc_area_node(area, gfp_mask, prot, shift, node);
> > if (!ret) {
> >   free_vm_area(area);
> >   goto fail;
> > }
> > ...
> > <snip>
> > 
> > vfree() - __also__ frees "vm_struct" where "area" points to. A NULL is
> > returned and free_vm_area() is invoked one more time on already freed
> > "area". 
> > 
> > Probably it is better to drop the below patch:
> > 
> > ff6f2b81eaa8a9fe5d158c6e7b1e58d3929c32c1 ("mm/vmalloc: move free_vm_area(area) from the __vmalloc_area_node function to the __vmalloc_node_range_noprof function")
> > 
> 
> If drop this commit, then the two “goto fail; ”in the __vmalloc_area_node function will cause area memory leaks in the __vmalloc_area_node function  when returning.
> 
It does not leak. On a fail case we release everything including "area":

fail:
    vfree(area->addr);
    return NULL;

this is how vfree() works.

> Perhaps the following changes should be added.
> 
> If the following changes should fix all issues I will send a new patch.
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 61981ee1c9d2..1826f3d70885 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3697,7 +3697,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
>                         warn_alloc(gfp_mask, NULL,
>                                 "vmalloc error: size %lu, failed to allocate pages",
>                                 area->nr_pages * PAGE_SIZE);
> -               goto fail;
> +               return NULL;
>         }
>  
>         /*
> @@ -3725,14 +3725,10 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
>                 warn_alloc(gfp_mask, NULL,
>                         "vmalloc error: size %lu, failed to map pages",
>                         area->nr_pages * PAGE_SIZE);
> -               goto fail;
> +               return NULL;
>         }
>  
>         return area->addr;
> -
> -fail:
> -       vfree(area->addr);
> -       return NULL;
>  }
>  
It is better to drop the patch. It does not fix anything, instead it
has introduced a degrade.

--
Uladzislau Rezki