Re: [PATCH 4/5] hibernation, KFENCE: explicitly map/unmap KFENCE pages

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

On 9/23/26 17:49, Mike Rapoport wrote:
> On Wed, Sep 23, 2026 at 12:31:26PM +0200, David Hildenbrand (Arm) wrote:
>> On 9/17/26 08:07, Mike Rapoport (Microsoft) wrote:
>>> The pages protected by KFENCE are removed from the direct map.
>>>
>>> safe_copy_page() temporarily maps and unmaps them using set_direct_map
>>> APIs, or, when the stars align, even using debug_pagealloc_map_pages().
>>>
>>> Neither of these APIs cares whether it is a KFENCE page and both blindly
>>> perform the update of the kernel page table for any non-present page.
>>
>> Agreed, we should route this through the actual mechanism that modified the
>> directmap in the first place.
>>
>>>
>>> Ability to use debug_pagealloc_map_pages() to remap KFENCE pages when both
>>> KFENCE and debug_pagealloc are enabled is an amusing coincidence.
>>>
>>> But with increasing appetite for using set_direct_map for hardening
>>> purposes, it becomes too big of a hammer to enable saving any non-present
>>> page in the hibernation image.
>>>
>>> Another gotcha is that loongarch that does not have a direct map at all
>>> advertises ARCH_HAS_SET_DIRECT_MAP to allow coexistence of KFENCE and
>>> hibernation.
>>>
>>> Extend KFENCE with a bitmap that tracks which pages are protected and
>>> provide kfence_force_mapping() and kfence_restore_mapping() APIs that allow
>>> forced mapping and unmapping of KFENCE pages.
>>>
>>> Use these APIs in hibernate_{map,unmap}_pages() for KFENCE pages.
>>>
>>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>> ---
>>>  include/linux/kfence.h  | 29 +++++++++++++++++++++++++++
>>>  kernel/power/snapshot.c |  7 +++++++
>>>  mm/kfence/core.c        | 52 +++++++++++++++++++++++++++++++++++++++++++++++--
>>>  3 files changed, 86 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/kfence.h b/include/linux/kfence.h
>>> index e5822f6e7f279..33a126cb6d1b7 100644
>>> --- a/include/linux/kfence.h
>>> +++ b/include/linux/kfence.h
>>> @@ -222,6 +222,32 @@ struct kmem_obj_info;
>>>  bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *slab);
>>>  #endif
>>>  
>>> +/**
>>> + * kfence_force_mapping() - make sure a KFENCE page is mapped
>>> + * @page: page to map
>>> + *
>>> + * Check whether @page is protected and map it if needed.
>>> + *
>>> + * Requires: is_kfence_address(page_address(page))
>>> + *
>>> + * Return:
>>> + * * false - failed to map @page
>>> + * * true - @page is mapped
>>> + */
>>> +bool kfence_force_mapping(struct page *page);
>>> +
>>> +/**
>>> + * kfence_restore_mapping() - restore mapping of a KFENCE page
>>
>>
>> The name is misleading. You are actually restoring that the page is
>> unmapped/protected?
>>
>> 	kfence_force_mapping vs. kfence_restore_mapping
>>
>> is confusing.
>>
>> Can we find a better pair of function names that describe what is actually
>> happening?
>>
>> Maybe something along the lines of
>>
>> 	kfence_prepare_copy_page
>>
>> 	kfence_finish_copy_page
>>
>> That rather expresses what the caller intends to do.
> 
> Naming is hard :-D
> 
> I feel that _copy_page() is to vague and adding hibernation there is to
> mouthful :(

Yeah, I wouldn't mention hibernation.



> 
> And these could be used outside of safe_copy_page() someday.
> 
> How about 
> 
> kfence_set_page_present()
> kfence_set_page_default()


Maybe spell out "direct_map" ?

kfence_set_direct_map_present()

kfence_set_direct_map_default()

But yeah, something along those lines is better.

-- 
Cheers,

David