Re: [PATCH 2/4] raid6: riscv: Fix NULL pointer dereference issue
From: Palmer Dabbelt
Date: Tue Jun 10 2025 - 18:03:16 EST
On Tue, 10 Jun 2025 03:12:32 PDT (-0700), zhangchunyan@xxxxxxxxxxx wrote:
When running the raid6 user-space test program on RISC-V QEMU, there's a
segmentation fault which seems caused by accessing a NULL pointer,
which is the pointer variable p/q in raid6_rvv*_gen/xor_syndrome_real(),
p/q should have been equal to dptr[x], but when I use GDB command to
see its value, which was 0x10 like below:
"
Program received signal SIGSEGV, Segmentation fault.
0x0000000000011062 in raid6_rvv2_xor_syndrome_real (disks=<optimized out>, start=0, stop=<optimized out>, bytes=4096, ptrs=<optimized out>) at rvv.c:386
(gdb) p p
$1 = (u8 *) 0x10 <error: Cannot access memory at address 0x10>
"
The issue was found to be related with:
1) Compile optimization
There's no segmentation fault if compiling the raid6test program with
the optimization flag -O0.
2) The RISC-V vector command vsetvli
If not used t0 as the first parameter in vsetvli, there's no
segmentation fault either.
This patch selects the 2nd solution to fix the issue.
This code is super fragile, it's got a bunch of vector asm blocks in
there that aren't declaring their cobbers. At a bare minimum we should
have something like
diff --git a/lib/raid6/rvv.c b/lib/raid6/rvv.c
index 99dfa16d37c7..3c9b3fd9f2ed 100644
--- a/lib/raid6/rvv.c
+++ b/lib/raid6/rvv.c
@@ -17,6 +17,10 @@
#define NSIZE 16
#endif
+#ifdef __riscv_vector
+#error "This code must be built without compiler support for vector"
+#endif
+
static void raid6_rvv1_gen_syndrome_real(int disks, unsigned long bytes, void **ptrs)
{
u8 **dptr = (u8 **)ptrs;
because it just won't work when built with a compiler that can use
vector instructions.
Fixes: 6093faaf9593 ("raid6: Add RISC-V SIMD syndrome and recovery calculations")
Signed-off-by: Chunyan Zhang <zhangchunyan@xxxxxxxxxxx>
---
lib/raid6/rvv.c | 48 ++++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/lib/raid6/rvv.c b/lib/raid6/rvv.c
index bf7d5cd659e0..b193ea176d5d 100644
--- a/lib/raid6/rvv.c
+++ b/lib/raid6/rvv.c
@@ -23,9 +23,9 @@ static int rvv_has_vector(void)
static void raid6_rvv1_gen_syndrome_real(int disks, unsigned long bytes, void **ptrs)
{
u8 **dptr = (u8 **)ptrs;
- unsigned long d;
- int z, z0;
u8 *p, *q;
+ unsigned long vl, d;
+ int z, z0;
z0 = disks - 3; /* Highest data disk */
p = dptr[z0 + 1]; /* XOR parity */
@@ -33,8 +33,9 @@ static void raid6_rvv1_gen_syndrome_real(int disks, unsigned long bytes, void **
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/* v0:wp0, v1:wq0, v2:wd0/w20, v3:w10 */
@@ -96,7 +97,7 @@ static void raid6_rvv1_xor_syndrome_real(int disks, int start, int stop,
{
u8 **dptr = (u8 **)ptrs;
u8 *p, *q;
- unsigned long d;
+ unsigned long vl, d;
int z, z0;
z0 = stop; /* P/Q right side optimization */
@@ -105,8 +106,9 @@ static void raid6_rvv1_xor_syndrome_real(int disks, int start, int stop,
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/* v0:wp0, v1:wq0, v2:wd0/w20, v3:w10 */
@@ -192,9 +194,9 @@ static void raid6_rvv1_xor_syndrome_real(int disks, int start, int stop,
static void raid6_rvv2_gen_syndrome_real(int disks, unsigned long bytes, void **ptrs)
{
u8 **dptr = (u8 **)ptrs;
- unsigned long d;
- int z, z0;
u8 *p, *q;
+ unsigned long vl, d;
+ int z, z0;
z0 = disks - 3; /* Highest data disk */
p = dptr[z0 + 1]; /* XOR parity */
@@ -202,8 +204,9 @@ static void raid6_rvv2_gen_syndrome_real(int disks, unsigned long bytes, void **
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/*
@@ -284,7 +287,7 @@ static void raid6_rvv2_xor_syndrome_real(int disks, int start, int stop,
{
u8 **dptr = (u8 **)ptrs;
u8 *p, *q;
- unsigned long d;
+ unsigned long vl, d;
int z, z0;
z0 = stop; /* P/Q right side optimization */
@@ -293,8 +296,9 @@ static void raid6_rvv2_xor_syndrome_real(int disks, int start, int stop,
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/*
@@ -410,9 +414,9 @@ static void raid6_rvv2_xor_syndrome_real(int disks, int start, int stop,
static void raid6_rvv4_gen_syndrome_real(int disks, unsigned long bytes, void **ptrs)
{
u8 **dptr = (u8 **)ptrs;
- unsigned long d;
- int z, z0;
u8 *p, *q;
+ unsigned long vl, d;
+ int z, z0;
z0 = disks - 3; /* Highest data disk */
p = dptr[z0 + 1]; /* XOR parity */
@@ -420,8 +424,9 @@ static void raid6_rvv4_gen_syndrome_real(int disks, unsigned long bytes, void **
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/*
@@ -536,7 +541,7 @@ static void raid6_rvv4_xor_syndrome_real(int disks, int start, int stop,
{
u8 **dptr = (u8 **)ptrs;
u8 *p, *q;
- unsigned long d;
+ unsigned long vl, d;
int z, z0;
z0 = stop; /* P/Q right side optimization */
@@ -545,8 +550,9 @@ static void raid6_rvv4_xor_syndrome_real(int disks, int start, int stop,
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/*
@@ -718,9 +724,9 @@ static void raid6_rvv4_xor_syndrome_real(int disks, int start, int stop,
static void raid6_rvv8_gen_syndrome_real(int disks, unsigned long bytes, void **ptrs)
{
u8 **dptr = (u8 **)ptrs;
- unsigned long d;
- int z, z0;
u8 *p, *q;
+ unsigned long vl, d;
+ int z, z0;
z0 = disks - 3; /* Highest data disk */
p = dptr[z0 + 1]; /* XOR parity */
@@ -728,8 +734,9 @@ static void raid6_rvv8_gen_syndrome_real(int disks, unsigned long bytes, void **
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/*
@@ -912,7 +919,7 @@ static void raid6_rvv8_xor_syndrome_real(int disks, int start, int stop,
{
u8 **dptr = (u8 **)ptrs;
u8 *p, *q;
- unsigned long d;
+ unsigned long vl, d;
int z, z0;
z0 = stop; /* P/Q right side optimization */
@@ -921,8 +928,9 @@ static void raid6_rvv8_xor_syndrome_real(int disks, int start, int stop,
asm volatile (".option push\n"
".option arch,+v\n"
- "vsetvli t0, x0, e8, m1, ta, ma\n"
+ "vsetvli %0, x0, e8, m1, ta, ma\n"
".option pop\n"
+ : "=&r" (vl)
);
/*