Skip to contentSkip to conversation

PREEMPT_RT_FULL: arm coredump fails for cpu >= 4

PATCH In review Frank Rowand 2 participants 4 messages Sep 28, 2011 – Oct 2, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 PREEMPT_RT_FULL: arm coredump fails for cpu >= 4

ARM can not use SPLIT_PTLOCK_CPUS if PREEMPT_RT_FULL because vectors_user_mapping() creates a VM_ALWAYSDUMP mapping of the vector page, but no ptl->lock has been allocated for the page. An attempt to coredump that page will result in a kernel NULL pointer dereference when follow_page() attempts to lock the page.

The call tree to the NULL pointer dereference is:

do_notify_resume()

      get_signal_to_deliver()
         do_coredump()
            elf_core_dump()
               get_dump_page()
                  __get_user_pages()
                     follow_page()
                        pte_offset_map_lock() <----- a #define
                           ...
                              rt_spin_lock()

This patch is needed only if mm-shrink-the-page-frame-to-rt-size.patch is applied.

---
mm/Kconfig | 9 9 + 0 - 0 !

 1 file changed, 9 insertions(+)
mm/Kconfig +9 -0
Index: b/mm/Kconfig
===================================================================
@@ -172,13 +172,22 @@ config PAGEFLAGS_EXTENDED
 # page_table_lock, so that faults on different parts of the user address
 # space can be handled with less contention: split it at this NR_CPUS.
 # Default to 4 for wider testing, though 8 might be more appropriate.
+#
 # ARM's adjust_pte (unused if VIPT) depends on mm-wide page_table_lock.
+#
+# ARM can not use SPLIT_PTLOCK_CPUS if PREEMPT_RT_FULL because
+# vectors_user_mapping() creates a VM_ALWAYSDUMP mapping of the vector page,
+# but no ptl->lock has been allocated for the page.  An attempt to coredump
+# that page will result in a kernel NULL pointer dereference when
+# follow_page() attempts to lock the page.
+#
 # PA-RISC 7xxx's spinlock_t would enlarge struct page from 32 to 44 bytes.
 # DEBUG_SPINLOCK and DEBUG_LOCK_ALLOC spinlock_t also enlarge struct page.
 #
 config SPLIT_PTLOCK_CPUS
 	int
 	default "999999" if ARM && !CPU_CACHE_VIPT
+	default "999999" if ARM && PREEMPT_RT_FULL
 	default "999999" if PARISC && !PA20
 	default "999999" if DEBUG_SPINLOCK || DEBUG_LOCK_ALLOC
 	default "4"
#1 lore raw
Quoted 11 lines · Frank Rowand

ARM can not use SPLIT_PTLOCK_CPUS if PREEMPT_RT_FULL because vectors_user_mapping() creates a VM_ALWAYSDUMP mapping of the vector page, but no ptl->lock has been allocated for the page. An attempt to coredump that page will result in a kernel NULL pointer dereference when follow_page() attempts to lock the page.

This patch is needed only if mm-shrink-the-page-frame-to-rt-size.patch is applied.

Yeah, vile hackery that is.. why isn't pgtable_page_ctor() called on those pages?

Not that I care too much about split_pte_lock on ARM, they're mostly all tiny machines anyway so the gain is marginal, but it would be good to find out why the pgtable constructor isn't called properly.

#2 lore raw
Quoted 10 lines · Peter Zijlstra
Quoted 9 lines · Frank Rowand

ARM can not use SPLIT_PTLOCK_CPUS if PREEMPT_RT_FULL because vectors_user_mapping() creates a VM_ALWAYSDUMP mapping of the vector page, but no ptl->lock has been allocated for the page. An attempt to coredump that page will result in a kernel NULL pointer dereference when follow_page() attempts to lock the page.

Quoted 5 lines
Quoted 2 lines

This patch is needed only if mm-shrink-the-page-frame-to-rt-size.patch is applied.

Yeah, vile hackery that is.. why isn't pgtable_page_ctor() called on those pages?

Yep, that is the question. I started fixing that yesterday, but realized I was going about it the wrong way, so I sent a first version of the patch that simply avoids the problem.

I'll be looking at whether I can fix it cleanly.

Quoted 5 lines

Not that I care too much about split_pte_lock on ARM, they're mostly all tiny machines anyway so the gain is marginal, but it would be good to find out why the pgtable constructor isn't called properly.

#3 lore raw
Quoted 14 lines · Frank Rowand
Quoted 13 lines · Peter Zijlstra
Quoted 11 lines · Frank Rowand

ARM can not use SPLIT_PTLOCK_CPUS if PREEMPT_RT_FULL because vectors_user_mapping() creates a VM_ALWAYSDUMP mapping of the vector page, but no ptl->lock has been allocated for the page. An attempt to coredump that page will result in a kernel NULL pointer dereference when follow_page() attempts to lock the page.

This patch is needed only if mm-shrink-the-page-frame-to-rt-size.patch is applied.

Quoted 13 lines
Quoted 2 lines

Yeah, vile hackery that is.. why isn't pgtable_page_ctor() called on those pages?

Yep, that is the question. I started fixing that yesterday, but realized I was going about it the wrong way, so I sent a first version of the patch that simply avoids the problem.

I'll be looking at whether I can fix it cleanly.

Quoted 4 lines

Not that I care too much about split_pte_lock on ARM, they're mostly all tiny machines anyway so the gain is marginal, but it would be good to find out why the pgtable constructor isn't called properly.

Patch Version 2

version 1 did not fix the underlying problem, but instead changed mm/Kconfig to prevent ARM from enabling SPLIT_PTLOCK_CPUS.

Properly initialize the ptl->lock for the ARM vector page.

Without this patch, ARM can not use SPLIT_PTLOCK_CPUS if PREEMPT_RT_FULL because vectors_user_mapping() creates a VM_ALWAYSDUMP mapping of the vector page (address 0xffff0000), but no ptl->lock has been allocated for the page. An attempt to coredump that page will result in a kernel NULL pointer dereference when follow_page() attempts to lock the page.

The call tree to the NULL pointer dereference is:

do_notify_resume()

      get_signal_to_deliver()
         do_coredump()
            elf_core_dump()
               get_dump_page()
                  __get_user_pages()
                     follow_page()
                        pte_offset_map_lock() <----- a #define
                           ...
                              rt_spin_lock()

The underlying problem is exposed by mm-shrink-the-page-frame-to-rt-size.patch.

---
arch/arm/kernel/process.c | 25 25 + 0 - 0 !

 1 file changed, 25 insertions(+)
arch/arm/kernel/process.c +25 -0
Index: b/arch/arm/kernel/process.c
===================================================================
@@ -484,6 +484,31 @@ unsigned long arch_randomize_brk(struct 
 }
 
 #ifdef CONFIG_MMU
+
+/*
+ * CONFIG_SPLIT_PTLOCK_CPUS results in a page->ptl lock.  If the lock is not
+ * initialized by pgtable_page_ctor() then a coredump of the vector page will
+ * fail.
+ */
+static int __init vectors_user_mapping_init_page(void)
+{
+	struct page *page;
+	unsigned long addr = 0xffff0000;
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+
+	pgd = pgd_offset_k(addr);
+	pud = pud_offset(pgd, addr);
+	pmd = pmd_offset(pud, addr);
+	page = pmd_page(*(pmd));
+
+	pgtable_page_ctor(page);
+
+	return 0;
+}
+late_initcall(vectors_user_mapping_init_page);
+
 /*
  * The vectors page is always readable from user space for the
  * atomic helpers and the signal restart code.  Let's declare a mapping