linux-kernel - Re: [PATCH] riscv: use local label names instead of global ones in assembly

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: <1fad8701-9667-487f-b356-45c6d07eeb49@ghiti.fr>
Date: Mon, 6 Jan 2025 09:52:23 +0100
From: Alexandre Ghiti <alex@...ti.fr>
To: Clément Léger <cleger@...osinc.com>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, linux-riscv@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Cc: Alexandre Ghiti <alexghiti@...osinc.com>
Subject: Re: [PATCH] riscv: use local label names instead of global ones in
 assembly

Hi Clément,

On 03/01/2025 15:17, Clément Léger wrote:
> Local labels should be prefix by '.L' or they'll be exported in the
> symbol table. Additionally, this messes up the backtrace by displaying
> an incorrect symbol:
>
>    ...
>    [   12.751810] [<ffffffff80441628>] _copy_from_user+0x28/0xc2
>    [   12.752035] [<ffffffff800152ca>] handle_misaligned_load+0x1ca/0x2fc
>    [   12.752310] [<ffffffff80a033e8>] do_trap_load_misaligned+0x24/0xee
>    [   12.752596] [<ffffffff80a0dcae>] _new_vmalloc_restore_context_a0+0xc2/0xce


Yes, I noticed this last week, thanks for looking into it. I would add a 
Fixes tag and merge that in the next -rc if possible as the above 
backtrace is "disturbing".


>
> After:
>    ...
>    [   10.243916] [<ffffffff804415e4>] _copy_from_user+0x28/0xc2
>    [   10.244026] [<ffffffff800152ca>] handle_misaligned_load+0x1ca/0x2fc
>    [   10.244150] [<ffffffff80a033a0>] do_trap_load_misaligned+0x24/0xee
>    [   10.244268] [<ffffffff80a0dc66>] handle_exception+0x146/0x152
>
> Signed-off-by: Clément Léger <cleger@...osinc.com>
>
> ---
>   arch/riscv/kernel/entry.S | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index c200d329d4bd..216581835eb0 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -23,21 +23,21 @@
>   	REG_S 	a0, TASK_TI_A0(tp)
>   	csrr 	a0, CSR_CAUSE
>   	/* Exclude IRQs */
> -	blt  	a0, zero, _new_vmalloc_restore_context_a0
> +	blt  	a0, zero, .Lnew_vmalloc_restore_context_a0
>   
>   	REG_S 	a1, TASK_TI_A1(tp)
>   	/* Only check new_vmalloc if we are in page/protection fault */
>   	li   	a1, EXC_LOAD_PAGE_FAULT
> -	beq  	a0, a1, _new_vmalloc_kernel_address
> +	beq  	a0, a1, .Lnew_vmalloc_kernel_address
>   	li   	a1, EXC_STORE_PAGE_FAULT
> -	beq  	a0, a1, _new_vmalloc_kernel_address
> +	beq  	a0, a1, .Lnew_vmalloc_kernel_address
>   	li   	a1, EXC_INST_PAGE_FAULT
> -	bne  	a0, a1, _new_vmalloc_restore_context_a1
> +	bne  	a0, a1, .Lnew_vmalloc_restore_context_a1
>   
> -_new_vmalloc_kernel_address:
> +.Lnew_vmalloc_kernel_address:
>   	/* Is it a kernel address? */
>   	csrr 	a0, CSR_TVAL
> -	bge 	a0, zero, _new_vmalloc_restore_context_a1
> +	bge 	a0, zero, .Lnew_vmalloc_restore_context_a1
>   
>   	/* Check if a new vmalloc mapping appeared that could explain the trap */
>   	REG_S	a2, TASK_TI_A2(tp)
> @@ -69,7 +69,7 @@ _new_vmalloc_kernel_address:
>   	/* Check the value of new_vmalloc for this cpu */
>   	REG_L	a2, 0(a0)
>   	and	a2, a2, a1
> -	beq	a2, zero, _new_vmalloc_restore_context
> +	beq	a2, zero, .Lnew_vmalloc_restore_context
>   
>   	/* Atomically reset the current cpu bit in new_vmalloc */
>   	amoxor.d	a0, a1, (a0)
> @@ -83,11 +83,11 @@ _new_vmalloc_kernel_address:
>   	csrw	CSR_SCRATCH, x0
>   	sret
>   
> -_new_vmalloc_restore_context:
> +.Lnew_vmalloc_restore_context:
>   	REG_L 	a2, TASK_TI_A2(tp)
> -_new_vmalloc_restore_context_a1:
> +.Lnew_vmalloc_restore_context_a1:
>   	REG_L 	a1, TASK_TI_A1(tp)
> -_new_vmalloc_restore_context_a0:
> +.Lnew_vmalloc_restore_context_a0:
>   	REG_L	a0, TASK_TI_A0(tp)
>   .endm
>   


You can add:

Reviewed-by: Alexandre Ghiti <alexghiti@...osinc.com>

Thanks,

Alex