[v3] MIPS: memset.S: Fix return of __clear_user from Lpartial_fixup
diff mbox series

Message ID -
State Accepted
Delegated to: James Hogan
Headers show
Series
  • [v3] MIPS: memset.S: Fix return of __clear_user from Lpartial_fixup
Related show

Commit Message

Matt Redfearn April 17, 2018, 2:52 p.m. UTC
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.

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: 
Suggested-by: James Hogan <>
Signed-off-by: Matt Redfearn <>

---

Changes in v3:
- Just fix the issue at hand

Changes in v2:
- Use James Hogan's suggestion of replacing t1 with a0 to get the
  correct remainder count.

 arch/mips/lib/memset.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

James Hogan April 17, 2018, 3:43 p.m. UTC | #1
On Tue, Apr 17, 2018 at 03:52:21PM +0100, Matt Redfearn wrote:
> 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.
> 
> 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: 
> Suggested-by: James Hogan <>
> Signed-off-by: Matt Redfearn <>

Applied, thanks
James

Patch
diff mbox series

diff --git a/arch/mips/lib/memset.S b/arch/mips/lib/memset.S
index 90bcdf1224ee..184819c1d5c8 100644
--- a/arch/mips/lib/memset.S
+++ b/arch/mips/lib/memset.S
@@ -252,7 +252,7 @@ 
 	PTR_L		t0, TI_TASK($28)
 	andi		a2, STORMASK
 	LONG_L		t0, THREAD_BUADDR(t0)
-	LONG_ADDU	a2, t1
+	LONG_ADDU	a2, a0
 	jr		ra
 	LONG_SUBU	a2, t0