Re: [PATCH] mm/khugepaged: don't map folios that are not uptodate in MADV_COLLAPSE

["David Hildenbrand (Arm)" <david@kernel.org>]

On 11/6/26 15:06, Pedro Falcato wrote:
> On Fri, Nov 06, 2026 at 01:29:31PM +0100, David Hildenbrand (Arm) wrote:
>> On 11/6/26 13:09, Kiryl Shutsemau wrote:
>>>
>>> Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
>>>
>>> Separately from this fix: nothing on the mapping side catches this.
>>> Every caller checks uptodate on its own (next_uptodate_folio(),
>>> filemap_fault(), shmem_get_folio()), and khugepaged was the one that
>>> went straight from filemap_lock_folio() to do_set_pmd().
>>>
>>> Should do_set_pmd() and set_pte_range() assert it? Something like
>>>
>>>         VM_WARN_ON_ONCE_FOLIO(folio->mapping && !folio_test_anon(folio) &&
>>>                               !folio_test_uptodate(folio), folio);
>>
>> I think so. But looking at these hard-to-trigger but in the end very nasty
>> problems, should we actually reject the operation and do a pr_warn_once() or
>> something?
>>
> 
> Yes. FWIW, this is something that's been going through my mind. Our page table
> manipulation APIs (and usage) are way too fragile and allow you to do the
> most insane things without ever realizing it (e.g you can clear a pte, set
> over a PTE, get-and-clear and discard the result, and do all of the above with
> no locks held, set a pte for a folio that's not uptodate, set a pte for a folio
> for which you don't have the lock, etc).
> 
> Some basic assertions (even if there's no easy way to back down from those)
> would be nice, or ideally yes a rejection.
> 
> I would just add that these very much should be VM_WARN_ON_ONCE instead of
> pr_warn_once(). pr_warn_once() will (AIUI) not be caught by fuzzers, at least
> not as easily.

Good point on the fuzzers.

-- 
Cheers,

David