| 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
| ||
|
Message-ID: <mhng-09771FCE-11E1-401A-81E4-EF482D65AC0B@palmerdabbelt-mac>
Date: Thu, 12 Jun 2025 12:30:38 -0700 (PDT)
From: Palmer Dabbelt <palmer@...belt.com>
To: zhangchunyan@...as.ac.cn
CC: Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu,
Alexandre Ghiti <alex@...ti.fr>, Charlie Jenkins <charlie@...osinc.com>, song@...nel.org, yukuai3@...wei.com,
linux-riscv@...ts.infradead.org, linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org, zhang.lyra@...il.com
Subject: Re: [PATCH 2/4] raid6: riscv: Fix NULL pointer dereference issue
On Tue, 10 Jun 2025 03:12:32 PDT (-0700), zhangchunyan@...as.ac.cn 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.
I'm picking this one up as a fix, with a some slight commit message
wording change to describe the clobber issue. It should show up on
fixes soon, assuming nothing goes off the rails you can base the next
version of the patch set on bc75552b80e6 ("raid6: riscv: Fix NULL
pointer dereference caused by a missing clobber").
On a related note: I think we have another bug if NSIZE doesn't line up
with bytes as we're not handling the tails. I'm not sure if that can
happen, as I don't really know this code.
Also: unless I'm missing something you can replace one one of the loads
in the loop with a move, which I assume will be faster on some systems.
> Fixes: 6093faaf9593 ("raid6: Add RISC-V SIMD syndrome and recovery calculations")
> Signed-off-by: Chunyan Zhang <zhangchunyan@...as.ac.cn>
> ---
> 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)
> );
>
> /*
Powered by blists - more mailing lists