All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: "David Hildenbrand (Arm)" <david@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Alexander Potapenko <glider@google.com>,
	Marco Elver <elver@google.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>, Len Brown <lenb@kernel.org>,
	Pavel Machek <pavel@kernel.org>,
	kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH 4/5] hibernation, KFENCE: explicitly map/unmap KFENCE pages
Date: Thu, 24 Sep 2026 19:02:35 +0300	[thread overview]
Message-ID: <arVJm8CiLG6vpy9-@kernel.org> (raw)
In-Reply-To: <d568a29b-5745-4214-96d3-1b635cd46c15@kernel.org>

On Thu, Sep 24, 2026 at 10:19:33AM +0200, David Hildenbrand (Arm) wrote:
> 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()

They are not necessarily in direct map :)
loongarch does not have direct map, but still supports kfence
 
> But yeah, something along those lines is better.
> 
> -- 
> Cheers,
> 
> David

-- 
Sincerely yours,
Mike.


  reply	other threads:[~2026-09-24 16:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17  6:07 [PATCH 0/5] hibernation: make safe_copy_page more robust and remove debug_pagealloc support Mike Rapoport (Microsoft)
2026-09-17  6:07 ` [PATCH 1/5] hibernation: make swsusp_page helpers static Mike Rapoport (Microsoft)
2026-09-23 10:08   ` David Hildenbrand (Arm)
2026-09-17  6:07 ` [PATCH 2/5] hibernation: ensure secretmem pages don't reach a snapshot Mike Rapoport (Microsoft)
2026-09-23 10:10   ` David Hildenbrand (Arm)
2026-09-23 15:38     ` Mike Rapoport
2026-09-17  6:07 ` [PATCH 3/5] hibernate: handle potential errors in hibernate_{map,unmap}_page() Mike Rapoport (Microsoft)
2026-09-23 10:13   ` David Hildenbrand (Arm)
2026-09-23 15:42     ` Mike Rapoport
2026-09-24  8:16       ` David Hildenbrand (Arm)
2026-09-17  6:07 ` [PATCH 4/5] hibernation, KFENCE: explicitly map/unmap KFENCE pages Mike Rapoport (Microsoft)
2026-09-23 10:31   ` David Hildenbrand (Arm)
2026-09-23 15:49     ` Mike Rapoport
2026-09-24  8:19       ` David Hildenbrand (Arm)
2026-09-24 16:02         ` Mike Rapoport [this message]
2026-09-24 18:49           ` David Hildenbrand (Arm)
2026-09-25  9:33             ` Mike Rapoport
2026-09-25 11:00               ` David Hildenbrand (Arm)
2026-09-17  6:07 ` [PATCH 5/5] hibernation: make hibernation unavailable when debug_pagealloc is on Mike Rapoport (Microsoft)
2026-09-23 10:24   ` David Hildenbrand (Arm)
2026-09-23  9:26 ` [PATCH 0/5] hibernation: make safe_copy_page more robust and remove debug_pagealloc support Mike Rapoport

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arVJm8CiLG6vpy9-@kernel.org \
    --to=rppt@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=pavel@kernel.org \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.