linux-kernel - Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4bc238eb-410b-46b1-98e1-4aaa35e34404@codethink.co.uk>
Date: Fri, 3 May 2024 13:19:47 +0100
From: Ben Dooks <ben.dooks@...ethink.co.uk>
To: Alexandre Ghiti <alex@...ti.fr>, Xiao Wang <xiao.w.wang@...el.com>,
 paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu
Cc: jerry.shih@...ive.com, nick.knight@...ive.com, ajones@...tanamicro.com,
 bjorn@...osinc.com, andy.chiu@...ive.com, viro@...iv.linux.org.uk,
 cleger@...osinc.com, alexghiti@...osinc.com, haicheng.li@...el.com,
 akira.tsukamoto@...il.com, linux-riscv@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] riscv: uaccess: Allow the last potential unrolled copy

On 03/05/2024 13:16, Alexandre Ghiti wrote:
> Hi Xiao,
> 
> On 13/03/2024 11:33, Xiao Wang wrote:
>> When the dst buffer pointer points to the last accessible aligned 
>> addr, we
>> could still run another iteration of unrolled copy.
>>
>> Signed-off-by: Xiao Wang <xiao.w.wang@...el.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 2e665f8f8fcc..1399d797d81b 100644
>> --- a/arch/riscv/lib/uaccess.S
>> +++ b/arch/riscv/lib/uaccess.S
>> @@ -103,7 +103,7 @@ SYM_FUNC_START(fallback_scalar_usercopy)
>>       fixup REG_S   t4,  7*SZREG(a0), 10f
>>       addi    a0, a0, 8*SZREG
>>       addi    a1, a1, 8*SZREG
>> -    bltu    a0, t0, 2b
>> +    bleu    a0, t0, 2b
>>       addi    t0, t0, 8*SZREG /* revert to original value */
>>       j    .Lbyte_copy_tail
> 
> 
> I agree it is still safe to continue for another word_copy here.
> 
> Reviewed-by: Alexandre Ghiti <alexghiti@...osinc.com>

Out of interest, has anyone checked if causing a schedule event during
this code breaks like the last time we had issues with the upstream
testing?

I did propose saving the state of the user-access flag in the task
struct but we mostly solved it by making sleeping functions stay
away from the address calculation. This of course may have been done
already or need to be done if three's long areas where the user-access
flags can be disabled (generally only a few drivers did this, so we
may not have come across the problem)

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html