While debugging a few problems with the misaligned access kselftest,
Alexandre discovered some crash with the current code. Indeed, some
misaligned access was done by the kernel using put_user(). This
was resulting in trap and a kernel crash since. The path was the
following:
user -> kernel -> access to user memory -> misaligned trap -> trap ->
kernel -> misaligned handling -> memcpy -> crash due to failed page fault
while in interrupt disabled section.

Last discussion about kernel misaligned handling and interrupt reenabling
were actually not to reenable interrupt when handling misaligned access
being done by kernel. The best solution being not to do any misaligned
accesses to userspace memory, we considered a few options:

- Remove any call to put/get_user() potientally doing misaligned
  accesses
- Do not do any misaligned accesses in put/get_user() itself

The second solution was the one chosen as there are too many callsite to
put/get_user() that could potentially do misaligned accesses. We tried
two approaches for that, either split access in two aligned accesses
(and do RMW for put_user()) or call copy_from/to_user() which does not
do any misaligned accesses. The later one was the simpler to implement
(although the performances are probably lower than split aligned
accesses but still way better than doing misaligned access emulation)
and allows to support what we wanted.

These commits are based on top of Alex dev/alex/get_user_misaligned_v1
branch.

Clément Léger (2):
  riscv: process: use unsigned int instead of unsigned long for
    put_user()
  riscv: uaccess: do not do misaligned accesses in get/put_user()

 arch/riscv/include/asm/uaccess.h | 28 ++++++++++++++++++++++------
 arch/riscv/kernel/process.c      |  2 +-
 2 files changed, 23 insertions(+), 7 deletions(-)

-- 
2.49.0

On 5/30/25 22:56, Clément Léger wrote:
> While debugging a few problems with the misaligned access kselftest,
> Alexandre discovered some crash with the current code. Indeed, some
> misaligned access was done by the kernel using put_user(). This
> was resulting in trap and a kernel crash since. The path was the
> following:
> user -> kernel -> access to user memory -> misaligned trap -> trap ->
> kernel -> misaligned handling -> memcpy -> crash due to failed page fault
> while in interrupt disabled section.
>
> Last discussion about kernel misaligned handling and interrupt reenabling
> were actually not to reenable interrupt when handling misaligned access
> being done by kernel. The best solution being not to do any misaligned
> accesses to userspace memory, we considered a few options:
>
> - Remove any call to put/get_user() potientally doing misaligned
>    accesses
> - Do not do any misaligned accesses in put/get_user() itself
>
> The second solution was the one chosen as there are too many callsite to
> put/get_user() that could potentially do misaligned accesses. We tried
> two approaches for that, either split access in two aligned accesses
> (and do RMW for put_user()) or call copy_from/to_user() which does not
> do any misaligned accesses. The later one was the simpler to implement
> (although the performances are probably lower than split aligned
> accesses but still way better than doing misaligned access emulation)
> and allows to support what we wanted.
>
> These commits are based on top of Alex dev/alex/get_user_misaligned_v1
> branch.
>
> Clément Léger (2):
>    riscv: process: use unsigned int instead of unsigned long for
>      put_user()
>    riscv: uaccess: do not do misaligned accesses in get/put_user()
>
>   arch/riscv/include/asm/uaccess.h | 28 ++++++++++++++++++++++------
>   arch/riscv/kernel/process.c      |  2 +-
>   2 files changed, 23 insertions(+), 7 deletions(-)


We also need to prevent unsafe routines to trigger misaligned accesses, 
I have a patch for this here 
https://github.com/linux-riscv/linux/commit/7c172121aeb235dedeb6f5e06740527530edd6af

Clément, can you add this one to the series please?

I have just  triggered a CI with those fixes on top of my sbi 3.0 branch.

Thanks,

Alex

On 31/05/2025 15:32, Alexandre Ghiti wrote:
> On 5/30/25 22:56, Clément Léger wrote:
>> While debugging a few problems with the misaligned access kselftest,
>> Alexandre discovered some crash with the current code. Indeed, some
>> misaligned access was done by the kernel using put_user(). This
>> was resulting in trap and a kernel crash since. The path was the
>> following:
>> user -> kernel -> access to user memory -> misaligned trap -> trap ->
>> kernel -> misaligned handling -> memcpy -> crash due to failed page fault
>> while in interrupt disabled section.
>>
>> Last discussion about kernel misaligned handling and interrupt reenabling
>> were actually not to reenable interrupt when handling misaligned access
>> being done by kernel. The best solution being not to do any misaligned
>> accesses to userspace memory, we considered a few options:
>>
>> - Remove any call to put/get_user() potientally doing misaligned
>>    accesses
>> - Do not do any misaligned accesses in put/get_user() itself
>>
>> The second solution was the one chosen as there are too many callsite to
>> put/get_user() that could potentially do misaligned accesses. We tried
>> two approaches for that, either split access in two aligned accesses
>> (and do RMW for put_user()) or call copy_from/to_user() which does not
>> do any misaligned accesses. The later one was the simpler to implement
>> (although the performances are probably lower than split aligned
>> accesses but still way better than doing misaligned access emulation)
>> and allows to support what we wanted.
>>
>> These commits are based on top of Alex dev/alex/get_user_misaligned_v1
>> branch.
>>
>> Clément Léger (2):
>>    riscv: process: use unsigned int instead of unsigned long for
>>      put_user()
>>    riscv: uaccess: do not do misaligned accesses in get/put_user()
>>
>>   arch/riscv/include/asm/uaccess.h | 28 ++++++++++++++++++++++------
>>   arch/riscv/kernel/process.c      |  2 +-
>>   2 files changed, 23 insertions(+), 7 deletions(-)
> 
> 
> We also need to prevent unsafe routines to trigger misaligned accesses,
> I have a patch for this here https://github.com/linux-riscv/linux/
> commit/7c172121aeb235dedeb6f5e06740527530edd6af
> 
> Clément, can you add this one to the series please?

Yep sure.

> 
> I have just  triggered a CI with those fixes on top of my sbi 3.0 branch.
> 
> Thanks,
> 
> Alex
> 
>