[REGRESSION]: [PATCH mm-hotfixes v5 3/5] x86/mm/pat: acquire init_mm read lock on attribute change to avoid UAF
Borah, Chaitanya Kumar
chaitanya.kumar.borah at intel.com
Mon Jul 27 02:00:57 PDT 2026
On 7/23/2026 7:48 PM, Lorenzo Stoakes (ARM) wrote:
> On Thu, Jul 23, 2026 at 12:00:42PM +0530, Borah, Chaitanya Kumar wrote:
>> Hello Lorenzo,
>>
>> On 7/17/2026 11:00 PM, Lorenzo Stoakes (ARM) wrote:
>>> A previous commit protected us against races between ptdump and CPA
>>> collapse, however one still exists between attribute changes and collapse
>>> as reported by Denis V. Lunev (linked).
>>>
>>> When an attribute change arises, a lockless page table walker obtains a PTE
>>> entry, which is later written to via set_pte_atomic():
>>>
>>> ...
>>> -> change_page_attr_set_clr()
>>> -> __change_page_attr_set_clr()
>>> -> __change_page_attr()
>>> -> _lookup_address_cpa()
>>> -> lookup_address_in_pgd_attr()
>>> -> [ lockless page table walker ]
>>> -> set_pte_atomic()
>>>
>>> There is nothing preventing a concurrent CPA collapse which can free the
>>> PTE that was retrieved here, resulting in a use-after-free.
>>>
>>> With the mmap write lock taken on init_mm over CPA collapse, we can now
>>> resolve this race by acquiring an mmap read lock on init_mm over
>>> __change_page_attr_set_clr().
>>>
>>> This locks across the whole operation over which the walk and the PTE entry
>>> write occurs, solving the race.
>>>
>>> It is safe to do this here, as no spinlocks are held upon entry to
>>> __change_page_attr_set_clr().
>>>
>>> The CPA_COLLAPSE flag is only set by set_memory_rox(), which exclusively
>>> operates upon vmalloc ranges, and on x86 only within the module mapping
>>> space.
>>>
>>> This is important, because some callers directly invoke
>>> __change_page_attr_set_clr(), bypassing this lock. However, none of these
>>> operate within the module mapping space.
>>>
>>> * cpa_process_alias() - a recursive helper called by
>>> __change_page_attr_set_clr().
>>> * __set_memory_enc_pgtable() - operates on the direct mapping and (via
>>> __vmbus_establish_gpadl()) the vmalloc mapping space.
>>> * __set_pages_[n]p() - called by set_direct_map_[invalid, default,
>>> valid]_noflush(), __kernel_map_pages() - operates on the direct map.
>>> * kernel_[un]map_pages_in_pgd() - operates on EFI ranges.
>>>
>>> This work is based upon Denis V. Lunev's excellent analysis of the bug with
>>> gratitude.
>>>
>>
>> This seems to be causing regression in our linux-next CI [1] on some of the
>> older machines (fi-elk-e7500, fi-ilk-650, fi-pnv-d510)
>
> Thanks for the report!
>
> A small note - it'd be useful for you to clearly point
> out the revision of next to make our life a bit easier :)
>
Ack.
> I can see from below in the stack and the URI it's next-20260720. There was a
> broken merge in linux-next for a while but it's fixed by 20260720 so that's not
> it.
>
>>
>> <4> [196.236867] ======================================================
>> <4> [196.236878] WARNING: possible circular locking dependency detected
>> <4> [196.236890] 7.2.0-rc4-next-20260720-next-20260720-g3fe08b9796f3+ #1
>> Tainted: G S U L
>> <4> [196.236906] ------------------------------------------------------
>> <4> [196.236916] core_hotunplug/5687 is trying to acquire lock:
>> <4> [196.236927] ffffffff839048b8 ((init_mm).mmap_lock){++++}-{4:4}, at:
>> change_page_attr_set_clr+0x10a/0x220
>
> This is the result of my change indeed.
>
>> <4> [196.236958]
>> but task is already holding lock:
>> <4> [196.236969] ffff88810c852238 (&vm->mutex){+.+.}-{4:4}, at:
>
> So this is a lock dependency issue as this is obviously an entirely
> different lock.
>
>> i915_ggtt_driver_release+0xab/0x260 [i915]
>> <4> [196.237572]
>> which lock already depends on the new lock.
>> <4> [196.237585]
>> the existing dependency chain (in reverse order) is:
>> <4> [196.237597]
>> -> #2 (&vm->mutex){+.+.}-{4:4}:
>> <4> [196.237617] i915_gem_shrinker_taints_mutex+0x35/0x70 [i915]
>> <4> [196.238267] i915_address_space_init+0x226/0x2a0 [i915]
>> <4> [196.238961] i915_ggtt_init_hw+0x2c/0x140 [i915]
>> <4> [196.239638] i915_driver_hw_probe+0x208/0x380 [i915]
>> <4> [196.240260] i915_driver_probe+0x113/0x5b0 [i915]
>> <4> [196.240883] i915_pci_probe+0xe0/0x1d0 [i915]
>
> (another small note - running scripts/decode_stacktrace.sh would be hugely
> useful as these offsets aren't so useful unless I have the exact kernel
> binary you have with debug symbols :)
>
We have not been asked for that yet but we can surely provide it if it
becomes absolutely necessary for debugging.
>>
>> Detailed log can be seen found in [2]
>>
>> We confirmed that reverting the patch solves the issue.
>>
>> Could you please check why the patch causes this regression and provide
>> a fix if necessary?
>>
>> Regards
>> Chaitanya
>>
>> [1] https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
>> [2] https://intel-gfx-ci.01.org/tree/linux-next/next-20260720/fi-ilk-650/igt@core_hotunplug@unbind-rebind.html
>
> <snip>
>
> So looking into it we have deadlock potential here:
>
> 1. fs_reclaim -> &vm->mutex
>
> < device init >
> -> i915_address_space_init()
> -> ...
> -> i915_gem_shrinker_taints_mutex()
> (mark the dependency in lockdep)
>
> This is presumably because the shrinker needs to lock this mutex.
>
> 2. init_mm -> fs_reclaim
>
> This was introduced by my patch and there we have:
>
> < caller needs to set memory, e.g. BPF >
> -> set_memory_rox()
> -> change_page_attr_set_clr()
> -> [acquire init_mm lock] <-- my change
> -> __change_page_attr_set_clr()
> -> __change_page_attr()
> -> split_large_page()
> -> pte_alloc_one_kernel()
> [ allocation ]
> < trigger direct reclaim >
>
> Thus init_mm -> fs_reclaim becasue in split_large_page() there's:
>
> if (!debug_pagealloc_enabled())
> spin_unlock(&cpa_lock);
> pte = pte_alloc_one_kernel(&init_mm); <-- allocate
> if (!debug_pagealloc_enabled())
> spin_lock(&cpa_lock);
> if (!pte)
> return -ENOMEM;
>
> Note the CPA lock dance, will come back to that as that's how to solve this
> issue...
>
> 3. &vm->mutex -> init_mm
>
> < device remove >
> -> i915_ggtt_driver_release()
> -> ggtt_cleanup_hw()
> -> [acquire &vm->mutex lock]
> -> ggtt->vm.cleanup == gmch_ggtt_remove()
> -> intel_gmch_remove()
> -> intel_gtt_teardown_scratch_page()
> -> set_pages_wb()
> -> set_memory_wb()
> -> _set_memory_wb()
> -> change_page_attr_clear()
> -> change_page_attr_set_clr()
> -> [acquire init_mm lock] <-- my change
>
> This &vm->mutex -> init_mm
>
> And so there is a circular dependency:
>
> &vm->mutex ---> init_mm
> ^ |
> \ v
> \--- fs_reclaim
>
> (lockdep should totally do diagrams like this :)
>
> In reality to hit this you'd need something to be doing direct reclaim on
> allocating a split page table during a CPA operation with a simultaneous
> contended init_mm lock.
>
> But this issue is broader than that really - we can't be holding init_mm
> over an allocation.
>
> split_large_page() calls __split_large_page() after dropping the CPA lock,
> which opens with a revalidation:
>
> spin_lock(&pgd_lock);
> /*
> * Check for races, another CPU might have split this page
> * up for us already:
> */
> tmp = _lookup_address_cpa(cpa, address, &level, &nx, &rw);
> if (tmp != kpte) {
> spin_unlock(&pgd_lock);
> return 1;
> }
>
> There's also revalidation in __change_page_attr() on every split:
>
> err = split_large_page(cpa, kpte, address);
> if (!err)
> goto repeat;
>
> (Where goto repeat entails a complete rewalk)
>
> So dropping the init_mm lock here should not result in any problems with
> being raced by a collapse and was something already accounted for.
>
> So the broad strokes of the fix is to eliminate the init_mm -> fs_reclaim
> dependency and thus break the dependency loop.
>
> I'll send a respin with this addressed.
>
Thank you for sending the fixes.
CI looks good now.
https://intel-gfx-ci.01.org/tree/linux-next/combined-alt.html?
==
Chaitanya
> Cheers, Lorenzo
More information about the linux-arm-kernel
mailing list