Re: [PATCH v8 14/21] mm/mmap: Avoid zeroing vma tree in mmap_region()

[Bert Karwatzki]

Am Mittwoch, dem 02.10.2024 um 14:23 +0100 schrieb Lorenzo Stoakes:
> On Wed, Oct 02, 2024 at 01:13:16PM GMT, Lorenzo Stoakes wrote:
> > On Tue, Oct 01, 2024 at 04:34:00AM GMT, Bert Karwatzki wrote:
> > > I just noticed (via a bisect between v6.11 and v6.12-rc1) that this patch
> > > (commit f8d112a4e657 in linux-next tree) leads to a severe memory corruption
> > > error under these (rather rare) circumstances:
> > > 1. Start a 32bit windows game via steam (which uses proton, steam's version of wine)
> > > 2. When starting the game you the proton version used has to be updated
> > >
> > > The effect is the following: The updating process of proton hangs and the game does
> > > not start and even after an exit from steam two processes remain, one of them at
> > > 100% CPU:
> > > $ ps aux | grep rundll
> > > bert      222638  1.7  0.1 2054868 87492 ?       Ss   23:14   0:01 C:\windows\syswow64\rundll32.exe setupapi,InstallHinfSection Wow64Install 128 \\?\Z:\mnt\data\.steam\debian-installation\steamapps\common\Proton - Experimental\files\share\wine\wine.inf
> > > bert      222639 99.8  0.0 2054868 2380 ?        R    23:14   1:01 C:\windows\syswow64\rundll32.exe setupapi,InstallHinfSection Wow64Install 128 \\?\Z:\mnt\data\.steam\debian-installation\steamapps\common\Proton - Experimental\files\share\wine\wine.inf
> > >
> > > When trying to kill those processes with "killall rundll32.exe", these error happen:
> >
> > [snip]
> >
> > Starting a new thread because lei is totally breaking with all these dmesg
> > logs and I'm struggling to be able to reply correctly.
> >
> > Sorry to make it hard to follow everyone but there we go.
> >
> > I have tried to recreate the exact series of anon mappings and it is not
> > triggering for me, so unfortunately I'm going to have to ask you to try
> > something else.
> >
> > This does sort of hint at it being maybe an unusual code path with a file
> > set (possibly...) - could you try the below patch on fresh next 1st oct?
> >
> > You can grep the dmesg for 'LJS' and just provide that if it triggers,
> > mostly I want to see if this (unusual) code path triggers. There shouldn't
> > be any spamming.
> >
> > Thanks!
> >
>
> [snip]
>
> Ugh trying this locally and trying to repro now (and not succeeding
> unfortunately), and I realise that _does_ spam because apparently it's very
> common with steam to be call_mmap()'ing things into VM_PFNMAP (who knew).
>
> Can you try this instead? Thanks!
>
> ----8<----
> From e69b8c05daa20921bd86ef604982297bd2ced663 Mon Sep 17 00:00:00 2001
> From: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> Date: Wed, 2 Oct 2024 13:04:55 +0100
> Subject: [PATCH] ljs: add hacky log output
>
> ---
>  mm/mmap.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index dd4b35a25aeb..e513eb3721a3 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1463,11 +1463,18 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
>  		 * vma again as we may succeed this time.
>  		 */
>  		if (unlikely(vm_flags != vma->vm_flags && vmg.prev)) {
> +			unsigned long ljs_start = vma->vm_start;
> +			unsigned long ljs_end = vma->vm_end;
> +
>  			vmg.flags = vma->vm_flags;
> +
>  			/* If this fails, state is reset ready for a reattempt. */
>  			merge = vma_merge_new_range(&vmg);
>
>  			if (merge) {
> +				pr_err("LJS: HIT CASE [%lx, %lx) -> [%lx, %lx) orig flags=[%lu] flags=[%lu]\n",
> +				       ljs_start, ljs_end, vma->vm_start, vma->vm_end, vm_flags, vma->vm_flags);
> +
>  				/*
>  				 * ->mmap() can change vma->vm_file and fput
>  				 * the original file. So fput the vma->vm_file
> --
> 2.46.2

This gives no output.

Bert Karwatzki