linux-kernel - [PATCH v2] MIPS: memset.S: Fix return of __clear_user from Lpartial

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1523973590-23356-1-git-send-email-matt.redfearn@mips.com>
Date:   Tue, 17 Apr 2018 14:59:50 +0100
From:   Matt Redfearn <matt.redfearn@...s.com>
To:     James Hogan <jhogan@...nel.org>, Ralf Baechle <ralf@...ux-mips.org>
CC:     <linux-mips@...ux-mips.org>,
        Matt Redfearn <matt.redfearn@...s.com>,
        <stable@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: [PATCH v2] MIPS: memset.S: Fix return of __clear_user from Lpartial_fixup

The __clear_user function is defined to return the number of bytes that
could not be cleared. From the underlying memset / bzero implementation
this means setting register a2 to that number on return. Currently if a
page fault is triggered within the memset_partial block, the value
loaded into a2 on return is meaningless.

The label .Lpartial_fixup\@ is jumped to on page fault. In order to work
out how many bytes failed to copy, the exception handler should find how
many bytes left in the partial block (andi a2, STORMASK), add that to
the partial block end address (a2), and subtract the faulting address to
get the remainder. Currently it incorrectly subtracts the partial block
start address (t1), which has additionally has been clobbered to
generate a jump target in memset_partial. Fix this by adding the block
end address instead.

Since this code is non-trivial to read, add comments to describe the
fault handling.

This issue was found with the following test code:
      int j, k;
      for (j = 0; j < 512; j++) {
        if ((k = clear_user(NULL, j)) != j) {
           pr_err("clear_user (NULL %d) returned %d\n", j, k);
        }
      }
Which now passes on Creator Ci40 (MIPS32) and Cavium Octeon II (MIPS64).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@...r.kernel.org
Suggested-by: James Hogan <jhogan@...nel.org>
Signed-off-by: Matt Redfearn <matt.redfearn@...s.com>

---

Changes in v2:
- Use James Hogan's suggestion of replacing t1 with a0 to get the
  correct remainder count.
- Add comments to .Lpartial_fixup to aid those who next try to deciper
  this code.

 arch/mips/lib/memset.S | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/lib/memset.S b/arch/mips/lib/memset.S
index 90bcdf1224ee..fa3bec269331 100644
--- a/arch/mips/lib/memset.S
+++ b/arch/mips/lib/memset.S
@@ -250,11 +250,11 @@

 .Lpartial_fixup\@:
 	PTR_L		t0, TI_TASK($28)
-	andi		a2, STORMASK
-	LONG_L		t0, THREAD_BUADDR(t0)
-	LONG_ADDU	a2, t1
+	andi		a2, STORMASK	/* #Bytes beyond partial block */
+	LONG_L		t0, THREAD_BUADDR(t0)	/* Get faulting address */
+	LONG_ADDU	a2, a0		/* Add end address of partial block */
 	jr		ra
-	LONG_SUBU	a2, t0
+	 LONG_SUBU	a2, t0		/* a2 = partial_end + #bytes - fault */

 .Llast_fixup\@:
 	jr		ra
-- 
2.7.4