Re: [PATCH] arch, mm: promote DEBUG_WX to CHECK_WX

[Mike Rapoport <rppt@kernel.org>]

On Sat, Sep 26, 2026 at 08:47:50AM +0200, Christophe Leroy (CS GROUP) wrote:
> Hi Mike,
> 
> Le 25/09/2026 à 11:53, Mike Rapoport (Microsoft) a écrit :
> > Verification that the kernel does not have writable + executable
> > mappings is about detecting security risks rather than a pure debug
> > feature.
> > 
> > Major distribution configurations enable it in their kernels as well as
> > defconfigs of most architectures that have ARCH_HAS_DEBUG_WX.
> > 
> > Rename relevant generic configuration options to use CHECK_WX and move
> > their definitions from mm/Kconfig.debug to mm/Kconfig.
> > 
> > For arm that does not widely enable it, only rename its variants of the
> > config options.
> > 
> > Enabling CHECK_WX adds a few kilobytes to the kernel binary and while
> > the added size can be slightly reduced with churny updates of
> > architecture implementations of ptdump, the core functionality takes
> > most of the added size. It cannot be moved to .init.text because the
> > verification has to happen after init sections are freed.
> > 
> > With this, make generic CHECK_WX default to STRICT_KERNEL_RWX while
> > still leaving users targeting small kernels the possibility to opt-out.
> 
> Looking at how it is done in powerpc I have some doubt with your reasoning.
> 
> ptdump_check_wx() will report regardless of CONFIG_DEBUG_WX:
> 
> 	if (st.wx_pages) {
> 		pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found\n",
> 			st.wx_pages);
> 
> 		return false;
> 	} else {
> 		pr_info("Checked W+X mappings: passed, no W+X pages found\n");
> 
> 		return true;
> 	}
> 
> The only difference is we won't get the WARN_ONCE():
> 
> 	WARN_ONCE(IS_ENABLED(CONFIG_DEBUG_WX),
> 		  "powerpc/mm: Found insecure W+X mapping at address %p/%pS\n",
> 		  (void *)st->start_address, (void *)st->start_address);
> 
> 
> And I believe a big fat warning like this is a debug option not to be
> enabled on production kernels.

It's arguable that this indicates a security risk and you want to see it in
production as well. And on many major distros DEBUG_WX is on, so you do
have it in production.
 
> So I think we should instead do:
> 
> diff --git a/include/linux/ptdump.h b/include/linux/ptdump.h
> index 240bd3bff18dd..714f63fb604a0 100644
> --- a/include/linux/ptdump.h
> +++ b/include/linux/ptdump.h
> @@ -33,7 +33,7 @@ bool ptdump_check_wx(void);
> 
>  static inline void debug_checkwx(void)
>  {
> -	if (IS_ENABLED(CONFIG_DEBUG_WX))
> +	if (IS_ENABLED(CONFIG_PTDUMP))

I don't think we should change that.
It's up to the architecture how they want to report it. If an architecture
wants to keep the warning only for debug it should convert WARN_ to
VM_WARN_.

What we actually should change is debug_checkwx name :)

>  		ptdump_check_wx();
>  }
> 
> 
> That way you should get (untested) the following warning but not the big fat
> debug WARN():
> 
>   Checked W+X mappings: FAILED, %lu W+X pages found
> 
> Christophe
> 

-- 
Sincerely yours,
Mike.