LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/4] __asm_copy_to-from_user: Fixes
@ 2021-07-20  8:49 Akira Tsukamoto
  2021-07-20  8:50 ` [PATCH 1/4] riscv: __asm_copy_to-from_user: Fix: overrun copy Akira Tsukamoto
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Akira Tsukamoto @ 2021-07-20  8:49 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Guenter Roeck, Geert Uytterhoeven,
	Albert Ou, Akira Tsukamoto, linux-riscv, linux-kernel

These are series for the fix reported by Guenter, Geert and Qiu.

One patch to fix overrun memory access, one patch to fix on rv32.
And two more for clean up and typos.

Have tested on qemu rv32, qemu rv64 and beaglev beta board.

Thanks for the report and instructions to reproduce the error on rv32.

Akira

Akira Tsukamoto (4):
  riscv: __asm_copy_to-from_user: Fix: overrun copy
  riscv: __asm_copy_to-from_user: Fix: fail on RV32
  riscv: __asm_copy_to-from_user: Remove unnecessary size check
  riscv: __asm_copy_to-from_user: Fix: Typos in comments

 arch/riscv/lib/uaccess.S | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] riscv: __asm_copy_to-from_user: Fix: overrun copy
  2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
@ 2021-07-20  8:50 ` Akira Tsukamoto
  2021-07-20  8:51 ` [PATCH 2/4] riscv: __asm_copy_to-from_user: Fix: fail on RV32 Akira Tsukamoto
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Akira Tsukamoto @ 2021-07-20  8:50 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Guenter Roeck, Geert Uytterhoeven,
	Albert Ou, linux-riscv, linux-kernel


There were two causes for the overrun memory access.

The threshold size was too small.
The aligning dst require one SZREG and unrolling word copy requires
8*SZREG, total have to be at least 9*SZREG.

Inside the unrolling copy, the subtracting -(8*SZREG-1) would make
iteration happening one extra loop. Proper value is -(8*SZREG).

Signed-off-by: Akira Tsukamoto <akira.tsukamoto@gmail.com>
---
 arch/riscv/lib/uaccess.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index bceb0629e440..8bbeca89a93f 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -35,7 +35,7 @@ ENTRY(__asm_copy_from_user)
 	/*
 	 * Use byte copy only if too small.
 	 */
-	li	a3, 8*SZREG /* size must be larger than size in word_copy */
+	li	a3, 9*SZREG /* size must be larger than size in word_copy */
 	bltu	a2, a3, .Lbyte_copy_tail
 
 	/*
@@ -75,7 +75,7 @@ ENTRY(__asm_copy_from_user)
 	 * a3 - a1 & mask:(SZREG-1)
 	 * t0 - end of aligned dst
 	 */
-	addi	t0, t0, -(8*SZREG-1) /* not to over run */
+	addi	t0, t0, -(8*SZREG) /* not to over run */
 2:
 	fixup REG_L   a4,        0(a1), 10f
 	fixup REG_L   a5,    SZREG(a1), 10f
@@ -97,7 +97,7 @@ ENTRY(__asm_copy_from_user)
 	addi	a1, a1, 8*SZREG
 	bltu	a0, t0, 2b
 
-	addi	t0, t0, 8*SZREG-1 /* revert to original value */
+	addi	t0, t0, 8*SZREG /* revert to original value */
 	j	.Lbyte_copy_tail
 
 .Lshift_copy:
-- 
2.17.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/4] riscv: __asm_copy_to-from_user: Fix: fail on RV32
  2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
  2021-07-20  8:50 ` [PATCH 1/4] riscv: __asm_copy_to-from_user: Fix: overrun copy Akira Tsukamoto
@ 2021-07-20  8:51 ` Akira Tsukamoto
  2021-07-20  9:49   ` Geert Uytterhoeven
  2021-07-20  8:52 ` [PATCH 3/4] riscv: __asm_copy_to-from_user: Remove unnecessary size check Akira Tsukamoto
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Akira Tsukamoto @ 2021-07-20  8:51 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Guenter Roeck, Geert Uytterhoeven,
	Albert Ou, linux-riscv, linux-kernel


Had a bug when converting bytes to bits when the cpu was rv32.

The a3 contains the number of bytes and multiple of 8
would be the bits. The LGREG is holding 2 for RV32 and 3 for
RV32, so to achieve multiple of 8 it must always be constant 3.
The 2 was mistakenly used for rv32.

Signed-off-by: Akira Tsukamoto <akira.tsukamoto@gmail.com>
---
 arch/riscv/lib/uaccess.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index 8bbeca89a93f..279876821969 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -125,7 +125,7 @@ ENTRY(__asm_copy_from_user)
 	 * t3 - prev shift
 	 * t4 - current shift
 	 */
-	slli	t3, a3, LGREG
+	slli	t3, a3, 3 /* converting bytes in a3 to bits */
 	li	a5, SZREG*8
 	sub	t4, a5, t3
 
-- 
2.17.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] riscv: __asm_copy_to-from_user: Remove unnecessary size check
  2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
  2021-07-20  8:50 ` [PATCH 1/4] riscv: __asm_copy_to-from_user: Fix: overrun copy Akira Tsukamoto
  2021-07-20  8:51 ` [PATCH 2/4] riscv: __asm_copy_to-from_user: Fix: fail on RV32 Akira Tsukamoto
@ 2021-07-20  8:52 ` Akira Tsukamoto
  2021-07-20  8:53 ` [PATCH 4/4] riscv: __asm_copy_to-from_user: Fix: Typos in comments Akira Tsukamoto
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Akira Tsukamoto @ 2021-07-20  8:52 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Guenter Roeck, Geert Uytterhoeven,
	Albert Ou, linux-riscv, linux-kernel


Clean up:

The size of 0 will be evaluated in the next step. Not
required here.

Signed-off-by: Akira Tsukamoto <akira.tsukamoto@gmail.com>
---
 arch/riscv/lib/uaccess.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index 279876821969..54d497a03164 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -30,7 +30,6 @@ ENTRY(__asm_copy_from_user)
 	 * t0 - end of uncopied dst
 	 */
 	add	t0, a0, a2
-	bgtu	a0, t0, 5f
 
 	/*
 	 * Use byte copy only if too small.
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/4] riscv: __asm_copy_to-from_user: Fix: Typos in comments
  2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
                   ` (2 preceding siblings ...)
  2021-07-20  8:52 ` [PATCH 3/4] riscv: __asm_copy_to-from_user: Remove unnecessary size check Akira Tsukamoto
@ 2021-07-20  8:53 ` Akira Tsukamoto
  2021-07-20  9:31 ` [PATCH 0/4] __asm_copy_to-from_user: Fixes Geert Uytterhoeven
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Akira Tsukamoto @ 2021-07-20  8:53 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Guenter Roeck, Geert Uytterhoeven,
	Albert Ou, linux-riscv, linux-kernel


Fixing typos and grammar mistakes and using more intuitive label
name.

Signed-off-by: Akira Tsukamoto <akira.tsukamoto@gmail.com>
---
 arch/riscv/lib/uaccess.S | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index 54d497a03164..63bc691cff91 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -33,19 +33,20 @@ ENTRY(__asm_copy_from_user)
 
 	/*
 	 * Use byte copy only if too small.
+	 * SZREG holds 4 for RV32 and 8 for RV64
 	 */
 	li	a3, 9*SZREG /* size must be larger than size in word_copy */
 	bltu	a2, a3, .Lbyte_copy_tail
 
 	/*
-	 * Copy first bytes until dst is align to word boundary.
+	 * Copy first bytes until dst is aligned to word boundary.
 	 * a0 - start of dst
 	 * t1 - start of aligned dst
 	 */
 	addi	t1, a0, SZREG-1
 	andi	t1, t1, ~(SZREG-1)
 	/* dst is already aligned, skip */
-	beq	a0, t1, .Lskip_first_bytes
+	beq	a0, t1, .Lskip_align_dst
 1:
 	/* a5 - one byte for copying data */
 	fixup lb      a5, 0(a1), 10f
@@ -54,7 +55,7 @@ ENTRY(__asm_copy_from_user)
 	addi	a0, a0, 1	/* dst */
 	bltu	a0, t1, 1b	/* t1 - start of aligned dst */
 
-.Lskip_first_bytes:
+.Lskip_align_dst:
 	/*
 	 * Now dst is aligned.
 	 * Use shift-copy if src is misaligned.
@@ -71,7 +72,6 @@ ENTRY(__asm_copy_from_user)
 	 *
 	 * a0 - start of aligned dst
 	 * a1 - start of aligned src
-	 * a3 - a1 & mask:(SZREG-1)
 	 * t0 - end of aligned dst
 	 */
 	addi	t0, t0, -(8*SZREG) /* not to over run */
@@ -106,7 +106,7 @@ ENTRY(__asm_copy_from_user)
 	 * For misaligned copy we still perform aligned word copy, but
 	 * we need to use the value fetched from the previous iteration and
 	 * do some shifts.
-	 * This is safe because reading less than a word size.
+	 * This is safe because reading is less than a word size.
 	 *
 	 * a0 - start of aligned dst
 	 * a1 - start of src
@@ -116,7 +116,7 @@ ENTRY(__asm_copy_from_user)
 	 */
 	/* calculating aligned word boundary for dst */
 	andi	t1, t0, ~(SZREG-1)
-	/* Converting unaligned src to aligned arc */
+	/* Converting unaligned src to aligned src */
 	andi	a1, a1, ~(SZREG-1)
 
 	/*
@@ -128,7 +128,7 @@ ENTRY(__asm_copy_from_user)
 	li	a5, SZREG*8
 	sub	t4, a5, t3
 
-	/* Load the first word to combine with seceond word */
+	/* Load the first word to combine with second word */
 	fixup REG_L   a5, 0(a1), 10f
 
 3:
@@ -160,7 +160,7 @@ ENTRY(__asm_copy_from_user)
 	 * a1 - start of remaining src
 	 * t0 - end of remaining dst
 	 */
-	bgeu	a0, t0, 5f
+	bgeu	a0, t0, .Lout_copy_user  /* check if end of copy */
 4:
 	fixup lb      a5, 0(a1), 10f
 	addi	a1, a1, 1	/* src */
@@ -168,7 +168,7 @@ ENTRY(__asm_copy_from_user)
 	addi	a0, a0, 1	/* dst */
 	bltu	a0, t0, 4b	/* t0 - end of dst */
 
-5:
+.Lout_copy_user:
 	/* Disable access to user memory */
 	csrc CSR_STATUS, t6
 	li	a0, 0
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] __asm_copy_to-from_user: Fixes
  2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
                   ` (3 preceding siblings ...)
  2021-07-20  8:53 ` [PATCH 4/4] riscv: __asm_copy_to-from_user: Fix: Typos in comments Akira Tsukamoto
@ 2021-07-20  9:31 ` Geert Uytterhoeven
  2021-07-20 14:19 ` Guenter Roeck
  2021-07-24  0:58 ` Palmer Dabbelt
  6 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-07-20  9:31 UTC (permalink / raw)
  To: Akira Tsukamoto
  Cc: Paul Walmsley, Palmer Dabbelt, Guenter Roeck, Albert Ou,
	linux-riscv, Linux Kernel Mailing List, Gabriel L. Somlo

Hi Tsukamoto-san,

On Tue, Jul 20, 2021 at 10:49 AM Akira Tsukamoto
<akira.tsukamoto@gmail.com> wrote:
> These are series for the fix reported by Guenter, Geert and Qiu.
>
> One patch to fix overrun memory access, one patch to fix on rv32.
> And two more for clean up and typos.
>
> Have tested on qemu rv32, qemu rv64 and beaglev beta board.
>
> Thanks for the report and instructions to reproduce the error on rv32.
>
> Akira
>
> Akira Tsukamoto (4):
>   riscv: __asm_copy_to-from_user: Fix: overrun copy
>   riscv: __asm_copy_to-from_user: Fix: fail on RV32
>   riscv: __asm_copy_to-from_user: Remove unnecessary size check
>   riscv: __asm_copy_to-from_user: Fix: Typos in comments
>
>  arch/riscv/lib/uaccess.S | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)

Thanks, RV32 (vexriscv) is booting fine again.

Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] riscv: __asm_copy_to-from_user: Fix: fail on RV32
  2021-07-20  8:51 ` [PATCH 2/4] riscv: __asm_copy_to-from_user: Fix: fail on RV32 Akira Tsukamoto
@ 2021-07-20  9:49   ` Geert Uytterhoeven
  2021-07-20 10:18     ` Akira Tsukamoto
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2021-07-20  9:49 UTC (permalink / raw)
  To: Akira Tsukamoto
  Cc: Paul Walmsley, Palmer Dabbelt, Guenter Roeck, Albert Ou,
	linux-riscv, Linux Kernel Mailing List

Hi Tsukamoto-san,

Thanks for your patch!

On Tue, Jul 20, 2021 at 10:51 AM Akira Tsukamoto
<akira.tsukamoto@gmail.com> wrote:
> Had a bug when converting bytes to bits when the cpu was rv32.
>
> The a3 contains the number of bytes and multiple of 8
> would be the bits. The LGREG is holding 2 for RV32 and 3 for
> RV32, so to achieve multiple of 8 it must always be constant 3.

RV64

> The 2 was mistakenly used for rv32.
>
> Signed-off-by: Akira Tsukamoto <akira.tsukamoto@gmail.com>

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] riscv: __asm_copy_to-from_user: Fix: fail on RV32
  2021-07-20  9:49   ` Geert Uytterhoeven
@ 2021-07-20 10:18     ` Akira Tsukamoto
  0 siblings, 0 replies; 10+ messages in thread
From: Akira Tsukamoto @ 2021-07-20 10:18 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: akira.tsukamoto, Paul Walmsley, Palmer Dabbelt, Guenter Roeck,
	Albert Ou, linux-riscv, Linux Kernel Mailing List

Hi Geert,

On 7/20/2021 6:49 PM, Geert Uytterhoeven wrote:
> Hi Tsukamoto-san,
> 
> Thanks for your patch!
> 
> On Tue, Jul 20, 2021 at 10:51 AM Akira Tsukamoto
> <akira.tsukamoto@gmail.com> wrote:
>> Had a bug when converting bytes to bits when the cpu was rv32.
>>
>> The a3 contains the number of bytes and multiple of 8
>> would be the bits. The LGREG is holding 2 for RV32 and 3 for
>> RV32, so to achieve multiple of 8 it must always be constant 3.
> 
> RV64

Thanks, the LGREG is holding 2 for RV32 and 3 for RV64 (not RV32).

Akira

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] __asm_copy_to-from_user: Fixes
  2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
                   ` (4 preceding siblings ...)
  2021-07-20  9:31 ` [PATCH 0/4] __asm_copy_to-from_user: Fixes Geert Uytterhoeven
@ 2021-07-20 14:19 ` Guenter Roeck
  2021-07-24  0:58 ` Palmer Dabbelt
  6 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2021-07-20 14:19 UTC (permalink / raw)
  To: Akira Tsukamoto, Paul Walmsley, Palmer Dabbelt,
	Geert Uytterhoeven, Albert Ou, linux-riscv, linux-kernel

On 7/20/21 1:49 AM, Akira Tsukamoto wrote:
> These are series for the fix reported by Guenter, Geert and Qiu.
> 
> One patch to fix overrun memory access, one patch to fix on rv32.
> And two more for clean up and typos.
> 
> Have tested on qemu rv32, qemu rv64 and beaglev beta board.
> 
> Thanks for the report and instructions to reproduce the error on rv32.
> 
> Akira
> 
> Akira Tsukamoto (4):
>    riscv: __asm_copy_to-from_user: Fix: overrun copy
>    riscv: __asm_copy_to-from_user: Fix: fail on RV32
>    riscv: __asm_copy_to-from_user: Remove unnecessary size check
>    riscv: __asm_copy_to-from_user: Fix: Typos in comments
> 
>   arch/riscv/lib/uaccess.S | 27 +++++++++++++--------------
>   1 file changed, 13 insertions(+), 14 deletions(-)
> 

For the series:

Tested-by: Guenter Roeck <linux@roeck-us.net>

Tested with qemu for both riscv32 and riscv64.

Thanks!

Guenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/4] __asm_copy_to-from_user: Fixes
  2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
                   ` (5 preceding siblings ...)
  2021-07-20 14:19 ` Guenter Roeck
@ 2021-07-24  0:58 ` Palmer Dabbelt
  6 siblings, 0 replies; 10+ messages in thread
From: Palmer Dabbelt @ 2021-07-24  0:58 UTC (permalink / raw)
  To: akira.tsukamoto
  Cc: Paul Walmsley, linux, geert, aou, akira.tsukamoto, linux-riscv,
	linux-kernel

On Tue, 20 Jul 2021 01:49:42 PDT (-0700), akira.tsukamoto@gmail.com wrote:
> These are series for the fix reported by Guenter, Geert and Qiu.
>
> One patch to fix overrun memory access, one patch to fix on rv32.
> And two more for clean up and typos.
>
> Have tested on qemu rv32, qemu rv64 and beaglev beta board.
>
> Thanks for the report and instructions to reproduce the error on rv32.
>
> Akira
>
> Akira Tsukamoto (4):
>   riscv: __asm_copy_to-from_user: Fix: overrun copy
>   riscv: __asm_copy_to-from_user: Fix: fail on RV32
>   riscv: __asm_copy_to-from_user: Remove unnecessary size check
>   riscv: __asm_copy_to-from_user: Fix: Typos in comments
>
>  arch/riscv/lib/uaccess.S | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)

Thanks, these are on fixes.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-07-24  0:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20  8:49 [PATCH 0/4] __asm_copy_to-from_user: Fixes Akira Tsukamoto
2021-07-20  8:50 ` [PATCH 1/4] riscv: __asm_copy_to-from_user: Fix: overrun copy Akira Tsukamoto
2021-07-20  8:51 ` [PATCH 2/4] riscv: __asm_copy_to-from_user: Fix: fail on RV32 Akira Tsukamoto
2021-07-20  9:49   ` Geert Uytterhoeven
2021-07-20 10:18     ` Akira Tsukamoto
2021-07-20  8:52 ` [PATCH 3/4] riscv: __asm_copy_to-from_user: Remove unnecessary size check Akira Tsukamoto
2021-07-20  8:53 ` [PATCH 4/4] riscv: __asm_copy_to-from_user: Fix: Typos in comments Akira Tsukamoto
2021-07-20  9:31 ` [PATCH 0/4] __asm_copy_to-from_user: Fixes Geert Uytterhoeven
2021-07-20 14:19 ` Guenter Roeck
2021-07-24  0:58 ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).