linux-kernel - Re: [PATCH v1] arm64: mm: Don't sleep in split_kernel_leaf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f6ee43c3-7175-48a5-a483-f89650ce02bc@kernel.org>
Date: Mon, 3 Nov 2025 19:06:24 +0100
From: "David Hildenbrand (Red Hat)" <david@...nel.org>
To: Ryan Roberts <ryan.roberts@....com>, catalin.marinas@....com,
 will@...nel.org, yang@...amperecomputing.com, ardb@...nel.org,
 dev.jain@....com, scott@...amperecomputing.com, cl@...two.org
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Guenter Roeck <groeck@...gle.com>
Subject: Re: [PATCH v1] arm64: mm: Don't sleep in split_kernel_leaf_mapping()
 when in atomic context

On 03.11.25 18:28, Ryan Roberts wrote:
> On 03/11/2025 15:38, David Hildenbrand (Red Hat) wrote:
>>
>>>    }
>>>
>>> +static inline bool force_pte_mapping(void)
>>> +{
>>> +    bool bbml2 = system_capabilities_finalized() ?
>>> +        system_supports_bbml2_noabort() : cpu_supports_bbml2_noabort();
>>
>> You are only moving this function. Still, there is some room for improvement I
>> want to point out :)
>>
>> bbml2 could be a const (or a helper function like bbml2_supported).
>>
>>> +
>>> +    return (!bbml2 && (rodata_full || arm64_kfence_can_set_direct_map() ||
>>> +               is_realm_world())) ||
>>> +        debug_pagealloc_enabled();
>>
>>
>> I suspect this could be made a bit easier to read.
>>
>>      if (debug_pagealloc_enabled())
>>          return true;
>>      if (bbml2)
>>          return false;
>>      return rodata_full || arm64_kfence_can_set_direct_map() || is_realm_world();
> 
> Yeah, I guess that's a bit nicer. I'd prefer to tidy it up in as separate commit
> though. (feel free ;-) )

Separate commit is fine (hoping you can do it once this lands :P ).

> 
>>
>>
>>> +}
>>> +
>>>    static DEFINE_MUTEX(pgtable_split_lock);
>>>
>>>    int split_kernel_leaf_mapping(unsigned long start, unsigned long end)
>>> @@ -723,6 +733,16 @@ int split_kernel_leaf_mapping(unsigned long start,
>>> unsigned long end)
>>>        if (!system_supports_bbml2_noabort())
>>>            return 0;
>>>
>>> +    /*
>>> +     * If the region is within a pte-mapped area, there is no need to try to
>>> +     * split. Additionally, CONFIG_DEBUG_PAGEALLOC and CONFIG_KFENCE may
>>> +     * change permissions from softirq context so for those cases (which are
>>> +     * always pte-mapped), we must not go any further because taking the
>>> +     * mutex below may sleep.
>>> +     */
>>> +    if (force_pte_mapping() || is_kfence_address((void *)start))
>>> +        return 0;
>>> +
>>
>> We're effectively performing two system_supports_bbml2_noabort() checks,
>> similarly in
>> arch_kfence_init_pool().
>>
>> I wonder if there is a clean way to avoid that.
> 
> I thought about this too. But system_supports_bbml2_noabort() is actually a
> magic alternatives patching thing; 

Makes sense, so likely just another nop in the final code.

> the code is updated so it's zero overhead. I
> decided this was the simplest and clearest way to do it. But I'm open to other
> ideas...

Given that we have two such call sequences, I was wondering if we could 
have a helper that better expresses+documents the desired semantics.

static bool pte_leaf_split_possible()
{
	/*
	 * !BBML2_NOABORT systems should never run into scenarios where
          * we would have to split. So exit early and let calling code
	 * detect it + raise a warning.
	 */
	if (!system_supports_bbml2_noabort())
		return false;
	return force_pte_mapping();
}

Something like that maybe.

-- 
Cheers

David