linux-kernel - Re: sparc64 MM regression...

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20180315.160030.601332550949971028.davem@davemloft.net>
Date:   Thu, 15 Mar 2018 16:00:30 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     nitin.m.gupta@...cle.com
Cc:     sparclinux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: sparc64 MM regression...

From: David Miller <davem@...emloft.net>
Date: Thu, 15 Mar 2018 15:28:15 -0400 (EDT)

> 
> So I bisected a userspace corruption regression down to commit:
> 
> commit a8e654f01cb725d0bfd741ebca1bf4c9337969cc                                                                                    
> Author: Nitin Gupta <nitin.m.gupta@...cle.com>                                                                                     
> Date:   Wed Jan 31 16:18:09 2018 -0800                                                                                             
>                                                                                                                                    
>     sparc64: update pmdp_invalidate() to return old pmd value                                                                      
> 
> The transformation is basically from a set_pte_at() call into
> an atomic cmpxchg64() loop to set the pmd.
> 
> The problem is that set_pmd_at() does more than just assign
> the pmd entry.
> 
> It also does some accounting and also queues up a batch TLB
> flush entry.
> 
> So the side effect of this change is that the TLB is never
> flushed for these changed PMDs, and thus the userland memory
> corruption I was seeing.

This seems to do the trick:

diff --git a/arch/sparc/mm/tlb.c b/arch/sparc/mm/tlb.c
index 847ddff..b5cfab7 100644
--- a/arch/sparc/mm/tlb.c
+++ b/arch/sparc/mm/tlb.c
@@ -163,13 +163,10 @@ static void tlb_batch_pmd_scan(struct mm_struct *mm, unsigned long vaddr,
 	pte_unmap(pte);
 }
 
-void set_pmd_at(struct mm_struct *mm, unsigned long addr,
-		pmd_t *pmdp, pmd_t pmd)
-{
-	pmd_t orig = *pmdp;
-
-	*pmdp = pmd;
 
+static void __set_pmd_acct(struct mm_struct *mm, unsigned long addr,
+			   pmd_t orig, pmd_t pmd)
+{
 	if (mm == &init_mm)
 		return;
 
@@ -219,6 +216,15 @@ void set_pmd_at(struct mm_struct *mm, unsigned long addr,
 	}
 }
 
+void set_pmd_at(struct mm_struct *mm, unsigned long addr,
+		pmd_t *pmdp, pmd_t pmd)
+{
+	pmd_t orig = *pmdp;
+
+	*pmdp = pmd;
+	__set_pmd_acct(mm, addr, orig, pmd);
+}
+
 static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
 		unsigned long address, pmd_t *pmdp, pmd_t pmd)
 {
@@ -227,6 +233,7 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
 	do {
 		old = *pmdp;
 	} while (cmpxchg64(&pmdp->pmd, old.pmd, pmd.pmd) != old.pmd);
+	__set_pmd_acct(vma->vm_mm, address, old, pmd);
 
 	return old;
 }