| 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
| ||
|
Message-ID: <b1fb982a-4936-4c18-aaad-3e8e8f61bccc@arm.com>
Date: Mon, 10 Mar 2025 13:48:53 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Mark Rutland <mark.rutland@....com>,
Anshuman Khandual <anshuman.khandual@....com>
Cc: linux-arm-kernel@...ts.infradead.org,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Ard Biesheuvel <ardb@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] arm64/mm: Create level specific section mappings in
map_range()
On 10/03/2025 10:55, Mark Rutland wrote:
> On Mon, Mar 10, 2025 at 11:58:12AM +0530, Anshuman Khandual wrote:
>> Currently PMD section mapping mask i.e PMD_TYPE_SECT is used while creating
>> section mapping at all page table levels except the last level. This works
>> fine as the section mapping masks are exactly the same (0x1UL) for all page
>> table levels.
>>
>> This will change in the future with D128 page tables that have unique skip
>> level values (SKL) required for creating section mapping at different page
>> table levels. Hence use page table level specific section mapping macros
>> instead of the common PMD_TYPE_SECT. While here also ensure that a section
>> mapping is only created on page table levels which could support that on a
>> given page size configuration otherwise fall back to create table entries.
>>
>> Cc: Catalin Marinas <catalin.marinas@....com>
>> Cc: Will Deacon <will@...nel.org>
>> Cc: Ryan Roberts <ryan.roberts@....com>
>> Cc: Ard Biesheuvel <ardb@...nel.org>
>> Cc: linux-kernel@...r.kernel.org
>> Cc: linux-arm-kernel@...ts.infradead.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
>> ---
>> This patch applies on v6.14-rc6
>>
>> Changes in V2:
>>
>> - Dropped PGD_TYPE_SECT macro and its instance from map_range()
>> - Create table entries on levels where section mapping is not possible
>
> On the last version, there was talk of an extant bug:
>
> https://lore.kernel.org/all/5f1b36e2-6455-44d9-97b0-253aefd5024f@arm.com/
>
> ... did that turn out to not be the case?
In the absence of Ard (or anyone else) telling me it's not a bug, then I'll
continue to assert that it is a bug, and should have a Fixes tag, and Cc stable.
>
>>
>> Changes in V1:
>>
>> https://lore.kernel.org/all/20250303041834.2796751-1-anshuman.khandual@arm.com/
>>
>> arch/arm64/kernel/pi/map_range.c | 38 +++++++++++++++++++++++++++++---
>> 1 file changed, 35 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/pi/map_range.c b/arch/arm64/kernel/pi/map_range.c
>> index 2b69e3beeef8..25a70c675c4d 100644
>> --- a/arch/arm64/kernel/pi/map_range.c
>> +++ b/arch/arm64/kernel/pi/map_range.c
>> @@ -11,6 +11,22 @@
>>
>> #include "pi.h"
>>
>> +static bool sect_supported(int level)
>
> What exactly is this function supposed to indicate?
>
> Due to the 'default' case, it'll return true for level-3 tables where
> sections/blocks aren't possible, but pages are. So maybe this is just
> misnamed, and wants to be something like "leaf_supported()" ?
I always thought a "section" was (outdated?) linux terminology for a leaf
mapping at any level. For my understanding, I think you're saying it
specifically means a leaf mapping at any level other than the last level, so it
is actually equivalent to a "block" mapping in the Arm ARM?
Either way, the intended semantic for this function is "is it valid to create a
leaf mapping at the specified level?" where leaf = block or page.
Thanks,
Ryan
>
>> +{
>> + switch (level) {
>> + case -1:
>> + return false;
>> + case 0:
>> + if (IS_ENABLED(CONFIG_ARM64_16K_PAGES) ||
>> + IS_ENABLED(CONFIG_ARM64_64K_PAGES))
>> + return false;
>> + else
>> + return true;
>
> Surely:
>
> case 0:
> return IS_ENABLED(CONFIG_ARM64_4K_PAGES);
>
> ... though that's only the case when LPA2 is supported and TCR_ELx.DS is
> set.
>
>> + default:
>> + return true;
>
> As above, what is this supposed to return for level-3 tables where
> sections/blocks aren't possible?
>
> Can we BUILD_BUG() for bogus levels?
>
>> + }
>> +}
>> +
>> /**
>> * map_range - Map a contiguous range of physical pages into virtual memory
>> *
>> @@ -44,13 +60,29 @@ void __init map_range(u64 *pte, u64 start, u64 end, u64 pa, pgprot_t prot,
>> * Set the right block/page bits for this level unless we are
>> * clearing the mapping
>> */
>> - if (protval)
>> - protval |= (level < 3) ? PMD_TYPE_SECT : PTE_TYPE_PAGE;
>> + if (protval && sect_supported(level)) {
>
> The existing code is trying to find the leaf prot for a given level of
> table, so as-is checking sect_supported() is at best misleading.
>
> If we're going to do something different below based on
> sect_supported(), I reckon this should be moved into the path where
> sect_supported() returned true, maybe factored out into a helper for
> clarity.
>
> Mark.
>
>> + switch (level) {
>> + case 3:
>> + protval |= PTE_TYPE_PAGE;
>> + break;
>> + case 2:
>> + protval |= PMD_TYPE_SECT;
>> + break;
>> + case 1:
>> + protval |= PUD_TYPE_SECT;
>> + break;
>> + case 0:
>> + protval |= P4D_TYPE_SECT;
>> + break;
>> + default:
>> + break;
>> + }
>> + }
>>
>> while (start < end) {
>> u64 next = min((start | lmask) + 1, PAGE_ALIGN(end));
>>
>> - if (level < 3 && (start | next | pa) & lmask) {
>> + if ((level < 3 && (start | next | pa) & lmask) || !sect_supported(level)) {
>> /*
>> * This chunk needs a finer grained mapping. Create a
>> * table mapping if necessary and recurse.
>> --
>> 2.25.1
>>
>>
Powered by blists - more mailing lists