linux-kernel - Re: [PATCH 3/4] mm: prevent KSM from completely breaking VMA merging

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: <0ba29e21-22f1-446f-a9fd-863c99904ce2@redhat.com>
Date: Mon, 19 May 2025 21:29:05 +0200
From: David Hildenbrand <david@...hat.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 "Liam R . Howlett" <Liam.Howlett@...cle.com>,
 Vlastimil Babka <vbabka@...e.cz>, Jann Horn <jannh@...gle.com>,
 Pedro Falcato <pfalcato@...e.de>, Xu Xin <xu.xin16@....com.cn>,
 Chengming Zhou <chengming.zhou@...ux.dev>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 3/4] mm: prevent KSM from completely breaking VMA merging

On 19.05.25 21:26, Lorenzo Stoakes wrote:
> On Mon, May 19, 2025 at 09:11:10PM +0200, David Hildenbrand wrote:
>> On 19.05.25 21:02, Lorenzo Stoakes wrote:
>>> On Mon, May 19, 2025 at 08:04:22PM +0200, David Hildenbrand wrote:
>>>>
>>>>>> +/*
>>>>>> + * Are we guaranteed no driver can change state such as to preclude KSM merging?
>>>>>> + * If so, let's set the KSM mergeable flag early so we don't break VMA merging.
>>>>>> + *
>>>>>> + * This is applicable when PR_SET_MEMORY_MERGE has been set on the mm_struct via
>>>>>> + * prctl() causing newly mapped VMAs to have the KSM mergeable VMA flag set.
>>>>>> + *
>>>>>> + * If this is not the case, then we set the flag after considering mergeability,
>>>>>> + * which will prevent mergeability as, when PR_SET_MEMORY_MERGE is set, a new
>>>>>> + * VMA will not have the KSM mergeability VMA flag set, but all other VMAs will,
>>>>>> + * preventing any merge.
>>>>>
>>>>> Hmmm, so an ordinary MAP_PRIVATE of any file (executable etc.) will get
>>>>> VM_MERGEABLE set but not be able to merge?
>>>>>
>>>>> Probably these are not often expected to be merged ...
>>>>>
>>>>> Preventing merging should really only happen because of VMA flags that
>>>>> are getting set: VM_PFNMAP, VM_MIXEDMAP, VM_DONTEXPAND, VM_IO.
>>>>>
>>>>>
>>>>> I am not 100% sure why we bail out on special mappings: all we have to
>>>>> do is reliably identify anon pages, and we should be able to do that.
>>>>>
>>>>> GUP does currently refuses any VM_PFNMAP | VM_IO, and KSM uses GUP,
>>>>> which might need a tweak then (maybe the solution could be to ... not
>>>>> use GUP but a folio_walk).
>>>>
>>>> Oh, someone called "David" already did that. Nice :)
>>>>
>>>> So we *should* be able to drop
>>>>
>>>> * VM_PFNMAP: we correctly identify CoWed pages
>>>> * VM_MIXEDMAP: we correctly identify CoWed pages
>>>> * VM_IO: should not affect CoWed pages
>>>> * VM_DONTEXPAND: no idea why that should even matter here
>>>
>>> I objected in the other thread but now realise I forgot we're talking about
>>> MAP_PRIVATE... So we can do the CoW etc. Right.
>>>
>>> Then we just need to be able to copy the thing on CoW... but what about
>>> write-through etc. cache settings? I suppose we don't care once CoW'd...
>>
>> Yes. It's ordinary kernel-managed memory.
> 
> Yeah, it's the CoW'd bit right? So it's fine.
> 
>>
>>>
>>> But is this common enough of a use case to be worth the hassle of checking this
>>> is all ok?
>>
>> The reason I bring it up is because
>>
>> 1) Just because some drivers do weird mmap() things, we cannot merge any
>> MAP_PRIVATE file mappings (except shmem ;) and mmap_prepare).
>>
>> 2) The whole "early_ksm" checks/handling would go away, making this patch
>> significantly simpler :)
> 
> OK you're starting to convince me...
> 
> Maybe this isn't such a big deal if the KSM code handles it already anwyay.
> 
> If you're sure GUP isn't relying on this... it could be an additional patch
> like:
> 
> 'remove VM_SPECIAL limitation, KSM can already handle this'
> 
> And probably we _should_ let any insane driver blow up if they change stupid
> things they must not change.

That is exactly my thinking.

But I'm also fine with doing that later, if you want to be careful.

-- 
Cheers,

David / dhildenb