All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ARM: mm: reduce maximum number of CPUs if DEBUG_KMAP_LOCAL is enabled
@ 2021-02-17 16:19 Ard Biesheuvel
  2021-02-18 14:08 ` Thomas Gleixner
  2021-03-02  9:22 ` Linus Walleij
  0 siblings, 2 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2021-02-17 16:19 UTC (permalink / raw
  To: linux-arm-kernel
  Cc: Peter Robinson, linus.walleij, tglx, linux, Ard Biesheuvel

The debugging code for kmap_local() doubles the number of per-CPU fixmap
slots allocated for kmap_local(), in order to use half of them as guard
regions. This causes the fixmap region to grow downwards beyond the start
of the fixmap region if the supported number of CPUs is large, and collide
with the newly added virtual DT mapping, which is obviously not good.

One manifestation of this is EFI boot on a kernel built with NR_CPUS=32
and CONFIG_DEBUG_KMAP_LOCAL=y, which may pass the FDT in highmem, resulting
in block entries below the fixmap region that the fixmap code misidentifies
as fixmap table entries, and subsequently tries to dereference using a
phys-to-virt translation that is only valid for lowmem. This results in a
cryptic splat such as the one below.

  ftrace: allocating 45548 entries in 89 pages
  8<--- cut here ---
  Unable to handle kernel paging request at virtual address fc6006f0
  pgd = (ptrval)
  [fc6006f0] *pgd=80000040207003, *pmd=00000000
  Internal error: Oops: a06 [#1] SMP ARM
  Modules linked in:
  CPU: 0 PID: 0 Comm: swapper Not tainted 5.11.0+ #382
  Hardware name: Generic DT based system
  PC is at cpu_ca15_set_pte_ext+0x24/0x30
  LR is at __set_fixmap+0xe4/0x118
  pc : [<c041ac9c>]    lr : [<c04189d8>]    psr: 400000d3
  sp : c1601ed8  ip : 00400000  fp : 00800000
  r10: 0000071f  r9 : 00421000  r8 : 00c00000
  r7 : 00c00000  r6 : 0000071f  r5 : ffade000  r4 : 4040171f
  r3 : 00c00000  r2 : 4040171f  r1 : c041ac78  r0 : fc6006f0
  Flags: nZcv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment none
  Control: 30c5387d  Table: 40203000  DAC: 00000001
  Process swapper (pid: 0, stack limit = 0x(ptrval))

So let's limit CONFIG_NR_CPUS to 16 when CONFIG_DEBUG_KMAP_LOCAL is in
effect. Also, fix the BUILD_BUG_ON() check that was supposed to catch this,
by checking whether the region grows below the start address rather than
above the end address.

Fixes: 2a15ba82fa6ca3f3 ("ARM: highmem: Switch to generic kmap atomic")
Reported-by: Peter Robinson <pbrobinson@gmail.com>
Tested-by: Peter Robinson <pbrobinson@gmail.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
v2: - fix BUILD_BUG_ON() check
    - add a Fixes: tag and Peter's Tested-by:
    - clarify the commit log a bit

 arch/arm/Kconfig  | 8 +++++++-
 arch/arm/mm/mmu.c | 3 +--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 138248999df7..3d2c684eab77 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1310,9 +1310,15 @@ config KASAN_SHADOW_OFFSET
 
 config NR_CPUS
 	int "Maximum number of CPUs (2-32)"
-	range 2 32
+	range 2 16 if DEBUG_KMAP_LOCAL
+	range 2 32 if !DEBUG_KMAP_LOCAL
 	depends on SMP
 	default "4"
+	help
+	  The maximum number of CPUs that the kernel can support.
+	  Up to 32 CPUs can be supported, or up to 16 if kmap_local()
+	  debugging is enabled, which uses half of the per-CPU fixmap
+	  slots as guard regions.
 
 config HOTPLUG_CPU
 	bool "Support for hot-pluggable CPUs"
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index c06ebfbc48c4..56c7954cb626 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -388,8 +388,7 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot)
 	pte_t *pte = pte_offset_fixmap(pmd_off_k(vaddr), vaddr);
 
 	/* Make sure fixmap region does not exceed available allocation. */
-	BUILD_BUG_ON(FIXADDR_START + (__end_of_fixed_addresses * PAGE_SIZE) >
-		     FIXADDR_END);
+	BUILD_BUG_ON(__fix_to_virt(__end_of_fixed_addresses) < FIXADDR_START);
 	BUG_ON(idx >= __end_of_fixed_addresses);
 
 	/* we only support device mappings until pgprot_kernel has been set */
-- 
2.30.0


_______________________________________________
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 v2] ARM: mm: reduce maximum number of CPUs if DEBUG_KMAP_LOCAL is enabled
  2021-02-17 16:19 [PATCH v2] ARM: mm: reduce maximum number of CPUs if DEBUG_KMAP_LOCAL is enabled Ard Biesheuvel
@ 2021-02-18 14:08 ` Thomas Gleixner
  2021-03-02  9:22 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Gleixner @ 2021-02-18 14:08 UTC (permalink / raw
  To: Ard Biesheuvel, linux-arm-kernel
  Cc: Peter Robinson, linus.walleij, linux, Ard Biesheuvel

On Wed, Feb 17 2021 at 17:19, Ard Biesheuvel wrote:
>
> Fixes: 2a15ba82fa6ca3f3 ("ARM: highmem: Switch to generic kmap atomic")
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Tested-by: Peter Robinson <pbrobinson@gmail.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Acked-by: Thomas Gleixner <tglx@linutronix.de>

_______________________________________________
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 v2] ARM: mm: reduce maximum number of CPUs if DEBUG_KMAP_LOCAL is enabled
  2021-02-17 16:19 [PATCH v2] ARM: mm: reduce maximum number of CPUs if DEBUG_KMAP_LOCAL is enabled Ard Biesheuvel
  2021-02-18 14:08 ` Thomas Gleixner
@ 2021-03-02  9:22 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2021-03-02  9:22 UTC (permalink / raw
  To: Ard Biesheuvel; +Cc: Thomas Gleixner, Russell King, Linux ARM, Peter Robinson

On Wed, Feb 17, 2021 at 5:19 PM Ard Biesheuvel <ardb@kernel.org> wrote:

> The debugging code for kmap_local() doubles the number of per-CPU fixmap
> slots allocated for kmap_local(), in order to use half of them as guard
> regions. This causes the fixmap region to grow downwards beyond the start
> of the fixmap region if the supported number of CPUs is large, and collide
> with the newly added virtual DT mapping, which is obviously not good.
>
> One manifestation of this is EFI boot on a kernel built with NR_CPUS=32
> and CONFIG_DEBUG_KMAP_LOCAL=y, which may pass the FDT in highmem, resulting
> in block entries below the fixmap region that the fixmap code misidentifies
> as fixmap table entries, and subsequently tries to dereference using a
> phys-to-virt translation that is only valid for lowmem. This results in a
> cryptic splat such as the one below.
>
>   ftrace: allocating 45548 entries in 89 pages
>   8<--- cut here ---
>   Unable to handle kernel paging request at virtual address fc6006f0
>   pgd = (ptrval)
>   [fc6006f0] *pgd=80000040207003, *pmd=00000000
>   Internal error: Oops: a06 [#1] SMP ARM
>   Modules linked in:
>   CPU: 0 PID: 0 Comm: swapper Not tainted 5.11.0+ #382
>   Hardware name: Generic DT based system
>   PC is at cpu_ca15_set_pte_ext+0x24/0x30
>   LR is at __set_fixmap+0xe4/0x118
>   pc : [<c041ac9c>]    lr : [<c04189d8>]    psr: 400000d3
>   sp : c1601ed8  ip : 00400000  fp : 00800000
>   r10: 0000071f  r9 : 00421000  r8 : 00c00000
>   r7 : 00c00000  r6 : 0000071f  r5 : ffade000  r4 : 4040171f
>   r3 : 00c00000  r2 : 4040171f  r1 : c041ac78  r0 : fc6006f0
>   Flags: nZcv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment none
>   Control: 30c5387d  Table: 40203000  DAC: 00000001
>   Process swapper (pid: 0, stack limit = 0x(ptrval))
>
> So let's limit CONFIG_NR_CPUS to 16 when CONFIG_DEBUG_KMAP_LOCAL is in
> effect. Also, fix the BUILD_BUG_ON() check that was supposed to catch this,
> by checking whether the region grows below the start address rather than
> above the end address.
>
> Fixes: 2a15ba82fa6ca3f3 ("ARM: highmem: Switch to generic kmap atomic")
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Tested-by: Peter Robinson <pbrobinson@gmail.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> v2: - fix BUILD_BUG_ON() check
>     - add a Fixes: tag and Peter's Tested-by:
>     - clarify the commit log a bit

Looks good to me.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

_______________________________________________
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:[~2021-03-02  9:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-17 16:19 [PATCH v2] ARM: mm: reduce maximum number of CPUs if DEBUG_KMAP_LOCAL is enabled Ard Biesheuvel
2021-02-18 14:08 ` Thomas Gleixner
2021-03-02  9:22 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.