[PATCH 0/3] Check for accesses beyond end of PGD

[PATCH 0/3] Check for accesses beyond end of PGD

[David Daney]

In 64-bit kernels a user space access might cause us to try to access
PGD elements beyound the end of the PGD, we must check for this and
trap it.

David Daney (3):
  MIPS:  Add uasm_i_dsrl_safe() and uasm_i_dsll_safe() to uasm.
  MIPS: Use uasm_i_ds{r,l}l_safe() instead of uasm_i_ds{r,l}l() in
    tlbex.c
  MIPS: Check for accesses beyond the end of the PGD.

 arch/mips/include/asm/uasm.h |   18 ++++++
 arch/mips/mm/tlbex.c         |  129 ++++++++++++++++++++++++++++++++----------
 2 files changed, 117 insertions(+), 30 deletions(-)

[PATCH 1/3] MIPS: Add uasm_i_dsrl_safe() and uasm_i_dsll_safe() to uasm.

[David Daney]

This allows us to clean up the code by not having to explicitly code
checks for shift amounts greater than 32.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 arch/mips/include/asm/uasm.h |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/mips/include/asm/uasm.h b/arch/mips/include/asm/uasm.h
index 11a8b52..697e40c 100644
--- a/arch/mips/include/asm/uasm.h
+++ b/arch/mips/include/asm/uasm.h
@@ -167,6 +167,24 @@ static inline void __cpuinit uasm_l##lb(struct uasm_label **lab, u32 *addr) \
 #define uasm_i_ssnop(buf) uasm_i_sll(buf, 0, 0, 1)
 #define uasm_i_ehb(buf) uasm_i_sll(buf, 0, 0, 3)
 
+static inline void uasm_i_dsrl_safe(u32 **p, unsigned int a1,
+				    unsigned int a2, unsigned int a3)
+{
+	if (a3 < 32)
+		uasm_i_dsrl(p, a1, a2, a3);
+	else
+		uasm_i_dsrl32(p, a1, a2, a3 - 32);
+}
+
+static inline void uasm_i_dsll_safe(u32 **p, unsigned int a1,
+				    unsigned int a2, unsigned int a3)
+{
+	if (a3 < 32)
+		uasm_i_dsll(p, a1, a2, a3);
+	else
+		uasm_i_dsll32(p, a1, a2, a3 - 32);
+}
+
 /* Handle relocations. */
 struct uasm_reloc {
 	u32 *addr;
-- 
1.6.6.1

Re: [PATCH 1/3] MIPS: Add uasm_i_dsrl_safe() and uasm_i_dsll_safe() to uasm.

[Ralf Baechle]

On Wed, Apr 28, 2010 at 12:16:16PM -0700, David Daney wrote:

> This allows us to clean up the code by not having to explicitly code
> checks for shift amounts greater than 32.

Thanks, applied.

  Ralf

[PATCH 2/3] MIPS: Use uasm_i_ds{r,l}l_safe() instead of uasm_i_ds{r,l}l() in tlbex.c

[David Daney]

This makes the code somewhat cleaner while reducing the risk of shift
amount overflows when various page table related options are changed.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 arch/mips/mm/tlbex.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index d1f68aa..61374b2 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -408,7 +408,7 @@ static __cpuinit __maybe_unused void build_convert_pte_to_entrylo(u32 **p,
 		UASM_i_ROTR(p, reg, reg, ilog2(_PAGE_GLOBAL) - ilog2(_PAGE_NO_EXEC));
 	} else {
 #ifdef CONFIG_64BIT_PHYS_ADDR
-		uasm_i_dsrl(p, reg, reg, ilog2(_PAGE_GLOBAL));
+		uasm_i_dsrl_safe(p, reg, reg, ilog2(_PAGE_GLOBAL));
 #else
 		UASM_i_SRL(p, reg, reg, ilog2(_PAGE_GLOBAL));
 #endif
@@ -549,14 +549,14 @@ build_get_pmde64(u32 **p, struct uasm_label **l, struct uasm_reloc **r,
 	 * SMTC uses TCBind value as "CPU" index
 	 */
 	uasm_i_mfc0(p, ptr, C0_TCBIND);
-	uasm_i_dsrl(p, ptr, ptr, 19);
+	uasm_i_dsrl_safe(p, ptr, ptr, 19);
 # else
 	/*
 	 * 64 bit SMP running in XKPHYS has smp_processor_id() << 3
 	 * stored in CONTEXT.
 	 */
 	uasm_i_dmfc0(p, ptr, C0_CONTEXT);
-	uasm_i_dsrl(p, ptr, ptr, 23);
+	uasm_i_dsrl_safe(p, ptr, ptr, 23);
 # endif
 	UASM_i_LA_mostly(p, tmp, pgdc);
 	uasm_i_daddu(p, ptr, ptr, tmp);
@@ -569,17 +569,15 @@ build_get_pmde64(u32 **p, struct uasm_label **l, struct uasm_reloc **r,
 
 	uasm_l_vmalloc_done(l, *p);
 
-	if (PGDIR_SHIFT - 3 < 32)		/* get pgd offset in bytes */
-		uasm_i_dsrl(p, tmp, tmp, PGDIR_SHIFT-3);
-	else
-		uasm_i_dsrl32(p, tmp, tmp, PGDIR_SHIFT - 3 - 32);
+	/* get pgd offset in bytes */
+	uasm_i_dsrl_safe(p, tmp, tmp, PGDIR_SHIFT - 3);
 
 	uasm_i_andi(p, tmp, tmp, (PTRS_PER_PGD - 1)<<3);
 	uasm_i_daddu(p, ptr, ptr, tmp); /* add in pgd offset */
 #ifndef __PAGETABLE_PMD_FOLDED
 	uasm_i_dmfc0(p, tmp, C0_BADVADDR); /* get faulting address */
 	uasm_i_ld(p, ptr, 0, ptr); /* get pmd pointer */
-	uasm_i_dsrl(p, tmp, tmp, PMD_SHIFT-3); /* get pmd offset in bytes */
+	uasm_i_dsrl_safe(p, tmp, tmp, PMD_SHIFT-3); /* get pmd offset in bytes */
 	uasm_i_andi(p, tmp, tmp, (PTRS_PER_PMD - 1)<<3);
 	uasm_i_daddu(p, ptr, ptr, tmp); /* add in pmd offset */
 #endif
@@ -720,9 +718,9 @@ static void __cpuinit build_update_entries(u32 **p, unsigned int tmp,
 			UASM_i_MTC0(p, tmp, C0_ENTRYLO0); /* load it */
 			UASM_i_ROTR(p, ptep, ptep, ilog2(_PAGE_GLOBAL) - ilog2(_PAGE_NO_EXEC));
 		} else {
-			uasm_i_dsrl(p, tmp, tmp, ilog2(_PAGE_GLOBAL)); /* convert to entrylo0 */
+			uasm_i_dsrl_safe(p, tmp, tmp, ilog2(_PAGE_GLOBAL)); /* convert to entrylo0 */
 			UASM_i_MTC0(p, tmp, C0_ENTRYLO0); /* load it */
-			uasm_i_dsrl(p, ptep, ptep, ilog2(_PAGE_GLOBAL)); /* convert to entrylo1 */
+			uasm_i_dsrl_safe(p, ptep, ptep, ilog2(_PAGE_GLOBAL)); /* convert to entrylo1 */
 		}
 		UASM_i_MTC0(p, ptep, C0_ENTRYLO1); /* load it */
 	} else {
@@ -793,9 +791,9 @@ static void __cpuinit build_r4000_tlb_refill_handler(void)
 		uasm_i_dmfc0(&p, K0, C0_BADVADDR);
 		uasm_i_dmfc0(&p, K1, C0_ENTRYHI);
 		uasm_i_xor(&p, K0, K0, K1);
-		uasm_i_dsrl32(&p, K1, K0, 62 - 32);
-		uasm_i_dsrl(&p, K0, K0, 12 + 1);
-		uasm_i_dsll32(&p, K0, K0, 64 + 12 + 1 - segbits - 32);
+		uasm_i_dsrl_safe(&p, K1, K0, 62);
+		uasm_i_dsrl_safe(&p, K0, K0, 12 + 1);
+		uasm_i_dsll_safe(&p, K0, K0, 64 + 12 + 1 - segbits);
 		uasm_i_or(&p, K0, K0, K1);
 		uasm_il_bnez(&p, &r, K0, label_leave);
 		/* No need for uasm_i_nop */
@@ -1322,9 +1320,9 @@ static void __cpuinit build_r4000_tlb_load_handler(void)
 		uasm_i_dmfc0(&p, K0, C0_BADVADDR);
 		uasm_i_dmfc0(&p, K1, C0_ENTRYHI);
 		uasm_i_xor(&p, K0, K0, K1);
-		uasm_i_dsrl32(&p, K1, K0, 62 - 32);
-		uasm_i_dsrl(&p, K0, K0, 12 + 1);
-		uasm_i_dsll32(&p, K0, K0, 64 + 12 + 1 - segbits - 32);
+		uasm_i_dsrl_safe(&p, K1, K0, 62);
+		uasm_i_dsrl_safe(&p, K0, K0, 12 + 1);
+		uasm_i_dsll_safe(&p, K0, K0, 64 + 12 + 1 - segbits);
 		uasm_i_or(&p, K0, K0, K1);
 		uasm_il_bnez(&p, &r, K0, label_leave);
 		/* No need for uasm_i_nop */
-- 
1.6.6.1

Re: [PATCH 2/3] MIPS: Use uasm_i_ds{r,l}l_safe() instead of uasm_i_ds{r,l}l() in tlbex.c

[Ralf Baechle]

On Wed, Apr 28, 2010 at 12:16:17PM -0700, David Daney wrote:

> This makes the code somewhat cleaner while reducing the risk of shift
> amount overflows when various page table related options are changed.

Applied.  Thanks!

  Ralf

[PATCH 3/3] MIPS: Check for accesses beyond the end of the PGD.

[David Daney]

For some combinations of PAGE_SIZE and vmbits, it is possible to have
userspace access that are beyond what is covered by the PGD, but
within vmbits.  Such an access would cause the TLB refill handler to
load garbage values for PMD and PTE potentially giving userspace
access to parts of the physical address space to which it is not
entitled.

In the TLB refill hot path, we add a single dsrl instruction so we can
check if any bits outside of the range covered by the PGD are set.  In
the vmalloc side we then separate the bad case from the normal vmalloc
case and call tlb_do_page_fault_0 if warranted.  This slows us down a
bit, but has the benefit of yielding deterministic behavior.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 arch/mips/mm/tlbex.c |   99 +++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 85 insertions(+), 14 deletions(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index 61374b2..8c8c6dd 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -31,6 +31,16 @@
 #include <asm/war.h>
 #include <asm/uasm.h>
 
+/*
+ * TLB load/store/modify handlers.
+ *
+ * Only the fastpath gets synthesized at runtime, the slowpath for
+ * do_page_fault remains normal asm.
+ */
+extern void tlb_do_page_fault_0(void);
+extern void tlb_do_page_fault_1(void);
+
+
 static inline int r45k_bvahwbug(void)
 {
 	/* XXX: We should probe for the presence of this bug, but we don't. */
@@ -83,6 +93,7 @@ enum label_id {
 	label_nopage_tlbm,
 	label_smp_pgtable_change,
 	label_r3000_write_probe_fail,
+	label_large_segbits_fault,
 #ifdef CONFIG_HUGETLB_PAGE
 	label_tlb_huge_update,
 #endif
@@ -101,6 +112,7 @@ UASM_L_LA(_nopage_tlbs)
 UASM_L_LA(_nopage_tlbm)
 UASM_L_LA(_smp_pgtable_change)
 UASM_L_LA(_r3000_write_probe_fail)
+UASM_L_LA(_large_segbits_fault)
 #ifdef CONFIG_HUGETLB_PAGE
 UASM_L_LA(_tlb_huge_update)
 #endif
@@ -157,6 +169,8 @@ static u32 tlb_handler[128] __cpuinitdata;
 static struct uasm_label labels[128] __cpuinitdata;
 static struct uasm_reloc relocs[128] __cpuinitdata;
 
+static int check_for_high_segbits __cpuinitdata;
+
 #ifndef CONFIG_MIPS_PGD_C0_CONTEXT
 /*
  * CONFIG_MIPS_PGD_C0_CONTEXT implies 64 bit and lack of pgd_current,
@@ -532,7 +546,24 @@ build_get_pmde64(u32 **p, struct uasm_label **l, struct uasm_reloc **r,
 	 * The vmalloc handling is not in the hotpath.
 	 */
 	uasm_i_dmfc0(p, tmp, C0_BADVADDR);
-	uasm_il_bltz(p, r, tmp, label_vmalloc);
+
+	if (check_for_high_segbits) {
+		/*
+		 * The kernel currently implicitely assumes that the
+		 * MIPS SEGBITS parameter for the processor is
+		 * (PGDIR_SHIFT+PGDIR_BITS) or less, and will never
+		 * allocate virtual addresses outside the maximum
+		 * range for SEGBITS = (PGDIR_SHIFT+PGDIR_BITS). But
+		 * that doesn't prevent user code from accessing the
+		 * higher xuseg addresses.  Here, we make sure that
+		 * everything but the lower xuseg addresses goes down
+		 * the module_alloc/vmalloc path.
+		 */
+		uasm_i_dsrl_safe(p, ptr, tmp, PGDIR_SHIFT + PGD_ORDER + PAGE_SHIFT - 3);
+		uasm_il_bnez(p, r, ptr, label_vmalloc);
+	} else {
+		uasm_il_bltz(p, r, tmp, label_vmalloc);
+	}
 	/* No uasm_i_nop needed here, since the next insn doesn't touch TMP. */
 
 #ifdef CONFIG_MIPS_PGD_C0_CONTEXT
@@ -583,28 +614,64 @@ build_get_pmde64(u32 **p, struct uasm_label **l, struct uasm_reloc **r,
 #endif
 }
 
+enum vmalloc64_mode {not_refill, refill};
 /*
  * BVADDR is the faulting address, PTR is scratch.
  * PTR will hold the pgd for vmalloc.
  */
 static void __cpuinit
 build_get_pgd_vmalloc64(u32 **p, struct uasm_label **l, struct uasm_reloc **r,
-			unsigned int bvaddr, unsigned int ptr)
+			unsigned int bvaddr, unsigned int ptr,
+			enum vmalloc64_mode mode)
 {
 	long swpd = (long)swapper_pg_dir;
+	int single_insn_swpd;
+	int did_vmalloc_branch = 0;
+
+	single_insn_swpd = uasm_in_compat_space_p(swpd) && !uasm_rel_lo(swpd);
 
 	uasm_l_vmalloc(l, *p);
 
-	if (uasm_in_compat_space_p(swpd) && !uasm_rel_lo(swpd)) {
-		uasm_il_b(p, r, label_vmalloc_done);
-		uasm_i_lui(p, ptr, uasm_rel_hi(swpd));
-	} else {
-		UASM_i_LA_mostly(p, ptr, swpd);
-		uasm_il_b(p, r, label_vmalloc_done);
-		if (uasm_in_compat_space_p(swpd))
-			uasm_i_addiu(p, ptr, ptr, uasm_rel_lo(swpd));
-		else
-			uasm_i_daddiu(p, ptr, ptr, uasm_rel_lo(swpd));
+	if (mode == refill && check_for_high_segbits) {
+		if (single_insn_swpd) {
+			uasm_il_bltz(p, r, bvaddr, label_vmalloc_done);
+			uasm_i_lui(p, ptr, uasm_rel_hi(swpd));
+			did_vmalloc_branch = 1;
+			/* fall through */
+		} else {
+			uasm_il_bgez(p, r, bvaddr, label_large_segbits_fault);
+		}
+	}
+	if (!did_vmalloc_branch) {
+		if (uasm_in_compat_space_p(swpd) && !uasm_rel_lo(swpd)) {
+			uasm_il_b(p, r, label_vmalloc_done);
+			uasm_i_lui(p, ptr, uasm_rel_hi(swpd));
+		} else {
+			UASM_i_LA_mostly(p, ptr, swpd);
+			uasm_il_b(p, r, label_vmalloc_done);
+			if (uasm_in_compat_space_p(swpd))
+				uasm_i_addiu(p, ptr, ptr, uasm_rel_lo(swpd));
+			else
+				uasm_i_daddiu(p, ptr, ptr, uasm_rel_lo(swpd));
+		}
+	}
+	if (mode == refill && check_for_high_segbits) {
+		uasm_l_large_segbits_fault(l, *p);
+		/*
+		 * We get here if we are an xsseg address, or if we are
+		 * an xuseg address above (PGDIR_SHIFT+PGDIR_BITS) boundary.
+		 *
+		 * Ignoring xsseg (assume disabled so would generate
+		 * (address errors?), the only remaining possibility
+		 * is the upper xuseg addresses.  On processors with
+		 * TLB_SEGBITS <= PGDIR_SHIFT+PGDIR_BITS, these
+		 * addresses would have taken an address error. We try
+		 * to mimic that here by taking a load/istream page
+		 * fault.
+		 */
+		UASM_i_LA(p, ptr, (unsigned long)tlb_do_page_fault_0);
+		uasm_i_jr(p, ptr);
+		uasm_i_nop(p);
 	}
 }
 
@@ -823,7 +890,7 @@ static void __cpuinit build_r4000_tlb_refill_handler(void)
 #endif
 
 #ifdef CONFIG_64BIT
-	build_get_pgd_vmalloc64(&p, &l, &r, K0, K1);
+	build_get_pgd_vmalloc64(&p, &l, &r, K0, K1, refill);
 #endif
 
 	/*
@@ -1300,7 +1367,7 @@ build_r4000_tlbchange_handler_tail(u32 **p, struct uasm_label **l,
 	uasm_i_eret(p); /* return from trap */
 
 #ifdef CONFIG_64BIT
-	build_get_pgd_vmalloc64(p, l, r, tmp, ptr);
+	build_get_pgd_vmalloc64(p, l, r, tmp, ptr, not_refill);
 #endif
 }
 
@@ -1524,6 +1591,10 @@ void __cpuinit build_tlb_refill_handler(void)
 	 */
 	static int run_once = 0;
 
+#ifdef CONFIG_64BIT
+	check_for_high_segbits = current_cpu_data.vmbits > (PGDIR_SHIFT + PGD_ORDER + PAGE_SHIFT - 3);
+#endif
+
 	switch (current_cpu_type()) {
 	case CPU_R2000:
 	case CPU_R3000:
-- 
1.6.6.1

Re: [PATCH 3/3] MIPS: Check for accesses beyond the end of the PGD.

[Ralf Baechle]

On Wed, Apr 28, 2010 at 12:16:18PM -0700, David Daney wrote:

Thanks, applied as well.

  Ralf