linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: mm: fix VA-range sanity check
@ 2023-06-15 10:26 Mark Rutland
  2023-06-15 11:23 ` Russell King (Oracle)
  2023-06-15 17:11 ` Catalin Marinas
  0 siblings, 2 replies; 3+ messages in thread
From: Mark Rutland @ 2023-06-15 10:26 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: catalin.marinas, linux, mark.rutland, steve.capper, will

Both create_mapping_noalloc() and update_mapping_prot() sanity-check the
their 'virt' parameter, but the check itself doesn't make much sense.
The condition used today appears to be a historical accident.

The sanity-check condition:

	if ((virt >= PAGE_END) && (virt < VMALLOC_START)) {
		[ ... warning here ... ]
		return;
	}

... can only be true for the KASAN shadow region or the module region,
and there's no reason to exclude these specifically for creating and
updateing mappings.

When arm64 support was first upstreamed in commit:

  c1cc1552616d0f35 ("arm64: MMU initialisation")

... the condition was:

	if (virt < VMALLOC_START) {
		[ ... warning here ... ]
		return;
	}

At the time, VMALLOC_START was the lowest kernel address, and this was
checking whether 'virt' would be translated via TTBR1.

Subsequently in commit:

  14c127c957c1c607 ("arm64: mm: Flip kernel VA space")

... the condition was changed to:

	if ((virt >= VA_START) && (virt < VMALLOC_START)) {
		[ ... warning here ... ]
		return;
	}

This appear to have been a thinko. The commit moved the linear map to
the bottom of the kernel address space, with VMALLOC_START being at the
halfway point. The old condition would warn for changes to the linear
map below this, and at the time VA_START was the end of the linear map.

Subsequently we cleaned up the naming of VA_START in commit:

  77ad4ce69321abbe ("arm64: memory: rename VA_START to PAGE_END")

... keeping the erroneous condition as:

	if ((virt >= PAGE_END) && (virt < VMALLOC_START)) {
		[ ... warning here ... ]
		return;
	}

Correct the condition to check against the start of the TTBR1 address
space, which is currently PAGE_OFFSET. This simplifies the logic, and
more clearly matches the "outside kernel range" message in the warning.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Steve Capper <steve.capper@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/mm/mmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 4829abe017e92..95d360805f8ae 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -451,7 +451,7 @@ static phys_addr_t pgd_pgtable_alloc(int shift)
 void __init create_mapping_noalloc(phys_addr_t phys, unsigned long virt,
 				   phys_addr_t size, pgprot_t prot)
 {
-	if ((virt >= PAGE_END) && (virt < VMALLOC_START)) {
+	if (virt < PAGE_OFFSET) {
 		pr_warn("BUG: not creating mapping for %pa at 0x%016lx - outside kernel range\n",
 			&phys, virt);
 		return;
@@ -478,7 +478,7 @@ void __init create_pgd_mapping(struct mm_struct *mm, phys_addr_t phys,
 static void update_mapping_prot(phys_addr_t phys, unsigned long virt,
 				phys_addr_t size, pgprot_t prot)
 {
-	if ((virt >= PAGE_END) && (virt < VMALLOC_START)) {
+	if (virt < PAGE_OFFSET) {
 		pr_warn("BUG: not updating mapping for %pa at 0x%016lx - outside kernel range\n",
 			&phys, virt);
 		return;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64: mm: fix VA-range sanity check
  2023-06-15 10:26 [PATCH] arm64: mm: fix VA-range sanity check Mark Rutland
@ 2023-06-15 11:23 ` Russell King (Oracle)
  2023-06-15 17:11 ` Catalin Marinas
  1 sibling, 0 replies; 3+ messages in thread
From: Russell King (Oracle) @ 2023-06-15 11:23 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, catalin.marinas, steve.capper, will

On Thu, Jun 15, 2023 at 11:26:28AM +0100, Mark Rutland wrote:
> Both create_mapping_noalloc() and update_mapping_prot() sanity-check the
> their 'virt' parameter, but the check itself doesn't make much sense.
> The condition used today appears to be a historical accident.
> 
> The sanity-check condition:
> 
> 	if ((virt >= PAGE_END) && (virt < VMALLOC_START)) {
> 		[ ... warning here ... ]
> 		return;
> 	}
> 
> ... can only be true for the KASAN shadow region or the module region,
> and there's no reason to exclude these specifically for creating and
> updateing mappings.
> 
> When arm64 support was first upstreamed in commit:
> 
>   c1cc1552616d0f35 ("arm64: MMU initialisation")
> 
> ... the condition was:
> 
> 	if (virt < VMALLOC_START) {
> 		[ ... warning here ... ]
> 		return;
> 	}
> 
> At the time, VMALLOC_START was the lowest kernel address, and this was
> checking whether 'virt' would be translated via TTBR1.
> 
> Subsequently in commit:
> 
>   14c127c957c1c607 ("arm64: mm: Flip kernel VA space")
> 
> ... the condition was changed to:
> 
> 	if ((virt >= VA_START) && (virt < VMALLOC_START)) {
> 		[ ... warning here ... ]
> 		return;
> 	}
> 
> This appear to have been a thinko. The commit moved the linear map to
> the bottom of the kernel address space, with VMALLOC_START being at the
> halfway point. The old condition would warn for changes to the linear
> map below this, and at the time VA_START was the end of the linear map.
> 
> Subsequently we cleaned up the naming of VA_START in commit:
> 
>   77ad4ce69321abbe ("arm64: memory: rename VA_START to PAGE_END")
> 
> ... keeping the erroneous condition as:
> 
> 	if ((virt >= PAGE_END) && (virt < VMALLOC_START)) {
> 		[ ... warning here ... ]
> 		return;
> 	}
> 
> Correct the condition to check against the start of the TTBR1 address
> space, which is currently PAGE_OFFSET. This simplifies the logic, and
> more clearly matches the "outside kernel range" message in the warning.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Will Deacon <will@kernel.org>

This simplifies the second of the kernel text replication patches!

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64: mm: fix VA-range sanity check
  2023-06-15 10:26 [PATCH] arm64: mm: fix VA-range sanity check Mark Rutland
  2023-06-15 11:23 ` Russell King (Oracle)
@ 2023-06-15 17:11 ` Catalin Marinas
  1 sibling, 0 replies; 3+ messages in thread
From: Catalin Marinas @ 2023-06-15 17:11 UTC (permalink / raw)
  To: linux-arm-kernel, Mark Rutland; +Cc: Will Deacon, linux, steve.capper

On Thu, 15 Jun 2023 11:26:28 +0100, Mark Rutland wrote:
> Both create_mapping_noalloc() and update_mapping_prot() sanity-check the
> their 'virt' parameter, but the check itself doesn't make much sense.
> The condition used today appears to be a historical accident.
> 
> The sanity-check condition:
> 
> 	if ((virt >= PAGE_END) && (virt < VMALLOC_START)) {
> 		[ ... warning here ... ]
> 		return;
> 	}
> 
> [...]

Applied to arm64 (for-next/misc), thanks!

[1/1] arm64: mm: fix VA-range sanity check
      https://git.kernel.org/arm64/c/ab9b4008092c

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-06-15 17:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-15 10:26 [PATCH] arm64: mm: fix VA-range sanity check Mark Rutland
2023-06-15 11:23 ` Russell King (Oracle)
2023-06-15 17:11 ` Catalin Marinas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).