[PATCH v1] riscv: uaccess: Report partial progress when the word copy faults

[PATCH v1] riscv: uaccess: Report partial progress when the word copy faults

[Nickolai Zeldovich]

write(2) from a buffer whose last 40 readable bytes are followed by an
unmapped page spins in generic_perform_write(), holding the inode lock,
until the task is killed, instead of returning 40; read(2) into such a
buffer fails with -EFAULT.

The unrolled word copy does eight loads before its eight stores, and
the shift copy loads one word ahead of its store, but their fixups
count everything from the first destination byte not stored as not
copied. So a fault in the first block reports that nothing was copied
although some bytes could be, which uaccess.h does not allow;
generic_perform_write() then retries forever, as
fault_in_iov_iter_readable() succeeds for the readable bytes.

Make those fixups finish the copy a byte at a time from the first byte
not stored; the byte copy's fixup then returns the exact count.

Reproduced on riscv64 under QEMU with write(2) to tmpfs and ramfs; the
patched kernel returns 40.

Fixes: 4b05b993900d ("riscv: uaccess: Return the number of bytes effectively not copied")
Signed-off-by: Nickolai Zeldovich <nickolai@csail.mit.edu>
---

Notes:
    This patch was prepared with the help of an AI tool.
    
    The scalar copy also runs when the vector copy is in use: for copies
    below riscv_v_usercopy_threshold and for the rest of a vector copy that
    faulted. Under QEMU -cpu max a 4096-byte write(2) from the buffer below
    hangs as well.
    
    Reproducer (riscv64, no privileges; the directory is on ramfs or tmpfs):
    
      gcc -O2 -o uaccess-progress uaccess-progress.c && ./uaccess-progress /tmp
    
    Before (mainline d135ab453a74, defconfig, QEMU -cpu max):
    "read:  -1 (expected 40)", then the write spins in the kernel until the
    alarm kills it ("Alarm clock"). After: "read:  40 (expected 40)" and
    "write: 40 (expected 40)".
    
      /*
       * read(2) and write(2) of 100 bytes from a buffer with 40 bytes before an
       * unmapped page: both must return 40.
       * Usage: uaccess-progress <dir on ramfs or tmpfs>
       */
      #include <fcntl.h>
      #include <stdio.h>
      #include <string.h>
      #include <sys/mman.h>
      #include <unistd.h>
    
      int main(int argc, char **argv)
      {
      	long pg = sysconf(_SC_PAGESIZE);
      	char path[256], *p;
      	ssize_t r;
      	int fd;
    
      	snprintf(path, sizeof(path), "%s/uaccess-progress", argc > 1 ? argv[1] : "/tmp");
      	p = mmap(NULL, 2 * pg, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
      	if (p == MAP_FAILED || munmap(p + pg, pg))
      		return 1;
      	memset(p, 'x', pg);
      	fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0600);
      	if (fd < 0 || pwrite(fd, p, pg, 0) != pg)
      		return 1;
    
      	r = pread(fd, p + pg - 40, 100, 0);
      	printf("read:  %zd (expected 40)\n", r);
      	fflush(stdout);
    
      	alarm(5);	/* the unpatched kernel never returns: "Alarm clock" */
      	r = pwrite(fd, p + pg - 40, 100, pg);
      	printf("write: %zd (expected 40)\n", r);
      	return 0;
      }

 arch/riscv/lib/uaccess.S | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index cf8586a937de8..99886ab86802e 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -88,9 +88,9 @@ SYM_FUNC_START(fallback_scalar_usercopy_sum_enabled)
 	beq	a0, t1, .Lskip_align_dst
 1:
 	/* a5 - one byte for copying data */
-	fixup lb      a5, 0(a1), 10f
+	fixup lb      a5, 0(a1), 11f
 	addi	a1, a1, 1	/* src */
-	fixup sb      a5, 0(a0), 10f
+	fixup sb      a5, 0(a0), 11f
 	addi	a0, a0, 1	/* dst */
 	bltu	a0, t1, 1b	/* t1 - start of aligned dst */
 
@@ -169,7 +169,7 @@ SYM_FUNC_START(fallback_scalar_usercopy_sum_enabled)
 	sub	t4, a5, t3
 
 	/* Load the first word to combine with second word */
-	fixup REG_L   a5, 0(a1), 10f
+	fixup REG_L   a5, 0(a1), 12f
 
 3:
 	/* Main shifting copy
@@ -181,11 +181,11 @@ SYM_FUNC_START(fallback_scalar_usercopy_sum_enabled)
 
 	/* At least one iteration will be executed */
 	srl	a4, a5, t3
-	fixup REG_L   a5, SZREG(a1), 10f
+	fixup REG_L   a5, SZREG(a1), 12f
 	addi	a1, a1, SZREG
 	sll	a2, a5, t4
 	or	a2, a2, a4
-	fixup REG_S   a2, 0(a0), 10f
+	fixup REG_S   a2, 0(a0), 13f
 	addi	a0, a0, SZREG
 	bltu	a0, t1, 3b
 
@@ -203,16 +203,31 @@ SYM_FUNC_START(fallback_scalar_usercopy_sum_enabled)
 	 */
 	bgeu	a0, t0, .Lout_copy_user  /* check if end of copy */
 4:
-	fixup lb      a5, 0(a1), 10f
+	fixup lb      a5, 0(a1), 11f
 	addi	a1, a1, 1	/* src */
-	fixup sb      a5, 0(a0), 10f
+	fixup sb      a5, 0(a0), 11f
 	addi	a0, a0, 1	/* dst */
 	bltu	a0, t0, 4b	/* t0 - end of dst */
 
 .Lout_copy_user:
 	li	a0, 0
 	ret
+
+	/*
+	 * A fault in the word or shift copy: redo the rest a byte at a time
+	 * from the first dst byte not stored, so that every byte before the
+	 * faulting one is copied and counted.
+	 */
+#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+13:	/* shift copy store: a1 is already one word ahead */
+	addi	a1, a1, -SZREG
+12:	/* shift copy load: the src of a0 is a1 + a3 */
+	add	a1, a1, a3
+#endif
 10:
+	mv	t0, t5
+	j	.Lbyte_copy_tail
+11:
 	sub a0, t5, a0
 	ret
 SYM_FUNC_END(fallback_scalar_usercopy_sum_enabled)

base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
-- 
2.53.0