Re: [PATCH] mm/secretmem: disable under HIGHMEM

[Mike Rapoport]

On Sun, Jul 05, 2026 at 10:46:19AM +0000, Brendan Jackman wrote:
> On Sun Jul 5, 2026 at 2:26 AM UTC, Andrew Morton wrote:
> >
> > Well OK, but the secretmem code is still wrong.  The patch protects
> > people from hitting the bug but leaves the bug in place.  Surely it would be
> > better to fix the bug?
> 
> I don't think the code is wrong if highmem is disabled. Certainly
> there is an implicit coupling between the .c file and the Kconfig file,
> but we could always add a BUILD_BUG_ON(IS_ENABLED(CONFIG_SECRETMEM)) to
> the relevant bit of code to make it explicit.
> 
> > Is that as simple as adding the folio_test_highmem() test?  
> 
> This would fix the WARN+SIGBUS but I don't think it resolves the fact
> that this configuration is completely untested - there are likely other
> functional bugs? But more importantly, I am not sure if secretmem
> actually does its security job if kmap_local_page() isn't a NOP. I
> think shipping a "security feature" that doesn't do what it says would
> be really terrible. (It might work totally fine, I dunno, but it would
> require some research and deep thinking that I don't really want to do
> for a configuration with no users).
> 
> > Or switching to GFP_KERNEL?  
> 
> ... Oh, that's a nice idea though :)

GFP_USER if anything :)

But still with kmap() and friends not being an NOP the promise "kernel does
not map this memory" does not hold.

I think that keeping SECRETMEM and HIGHMEM mutually exclusive is
conceptually correct.
 
> Any thoughts from Mike on that? I think it might be just as good as this
> patch? And then you can still use secretmem reliably on a 32bit build 
> as long as you have <1G RAM (or whatever the limit is).

With <1G RAM there is no need for HIGHMEM :) 

-- 
Sincerely yours,
Mike.