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
