linux-kernel - Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.0901301159500.3150@localhost.localdomain>
Date:	Fri, 30 Jan 2009 12:31:13 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Lee Schermerhorn <Lee.Schermerhorn@...com>
cc:	Hugh Dickins <hugh@...itas.com>, Greg KH <gregkh@...e.de>,
	Maksim Yevmenkin <maksim.yevmenkin@...il.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Nick Piggin <npiggin@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	will@...wder-design.com, Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Subject: Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM_LOCKED
 file segments

On Fri, 30 Jan 2009, Lee Schermerhorn wrote:
>
> Ad hoc instrumentation shows that it's the VM_ACCOUNT flag that is
> different between the existing file segment and the one attempting the
> merge:

Gaah. I looked at VM_ACCOUNT, since it looked like a prime example of the 
same VM_CAN_NONLINEAR thing and was thinking that I should just add it to 
the "ok to merge" flags, but decided after a quick look that we do all the 
VM_ACCOUNT handling before we call vma_merge, so I just left it at that. 

But apparently we _do_ do some VM_ACCOUNT stuff afterwards.

> is_mergeable_vma: !mergable: vma flags:  0x80020f9:0x1020f9
>                                            |         |-VM_ACCOUNT
>                                            +-----------VM_CAN_NONLINEAR
> 
> So happens, I'm mapping with MAP_SHARED, so the VM_ACCOUNT flag gets
> cleared later in mmap_region().  Comments say that this is for checking
> memory availability during shmem_file_setup().  Maybe we can move the
> temporary setting of VM_ACCOUNT until just before the call to
> shmem_zero_setup()?

Yeah, that would probably fix it, and looks like the right thing to do. 

It all looks pretty confused wrong to set the whole VM_ACCOUNT flag for a 
file-backed file AT ALL in the first place, but the code knows that it 
won't matter for a shared file, and will be cleared again later.

So it plays these temporary games with vm_flags, and it didn't matter 
because of how we used to call "vma_merge()" either early only for the 
anonymous memory case (that had VM_ACCOUNT stable and didn't have that 
temporary case at all) or much later (after having undone the temporary 
flag setting) for files.

Why do we pass in that "accountable" flag, btw? It's only ever set to 0 by 
a MAP_PRIVATE mapping that hits is_file_hugepages() (see do_mmap_pgoff), 
and we could just do that decision all inside mmap_region(). So the flag 
doesn't really seem to have any real meaning, and is just passed around 
for some odd historical reason?

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/