Re: [PATCH] Fix incorrect flush address in direct page table reclaim

On 8/4/26 11:25, Qi Zheng wrote:
> 
> 
> On 8/4/26 5:05 PM, David Hildenbrand (Arm) wrote:
>> On 8/4/26 02:37, [email protected] wrote:
>>> From: Andy Lutomirski <[email protected]>
>>>
>>> When zap_pte_range reclaims a page table, it does:
>>>
>>>      pte_free_tlb(tlb, pmd_pgtable(pmdval), addr);
>>>
>>> and this is unconditionally wrong: if this code executes, addr *always*
>>> points one past the end of the range covered by the table.  The addr
>>> parameter is used to flush the TLB (really the paging-structure-cache)
>>> to drop references to the to-be-freed table, and any architecture that
>>> cares about the parameter will flush the wrong address.  (But they'll
>>> still free the correct page).
>>>
>>> I think it's worth contemplating why the kernel works at all.
>>
>> Note that this only triggers when someone does e.g., a MADV_DONTNEED over
>> a large enough range (covering at least a full PTE table).
>>
>> So this isn't the ordinary munmap()/exit() page table reclaim code.
>>
>> I'm still surprised that it took so long to show up; likely we need more
>> targeted tests for PT_RECLAIM that
> 
> We backported the PT_RECLAIM to our internal tree a while ago (excluding
> the rework patch being fixed here), and it has been running stably ever
> since.

I'm wondering whether we should have a selftest that triggers this exact scenario:

(a) Populate page table(s)
(b) MADV_DONTNEED to reclaim the page table(s)
(c) Allocate plenty of pages for another purpose (draining PCP where the page
    table likely ended up) and fill them with some data that looks like present page
    table entries.
(d) Re-access the reclaimed VA space, expecting re-population of anonymous
    pages.

I can find someone to work on that.

Any other ideas what we could check? We could certainly also check that the page table
was actually reclaimed during MADV_DONTNEED.

-- 
Cheers,

David