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

[David Hildenbrand (Arm)]

On 7/5/26 13:34, Mike Rapoport wrote:
> 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 :)

Right.

> 
> 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.

We could even limit it to 64BIT ;)

-- 
Cheers,

David