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

Lorenzo Stoakes
2024-10-02T13:23:42Z

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@oracle.com>
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