linux-kernel - Re: [PATCH v2] LoongArch: KGDB: Rework arch_kgdb

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAAhV-H71aEKj2V8mAqbUuAe1JiHngWHW3rSaJ_Dx_CzoQC7TgQ@mail.gmail.com>
Date: Sat, 22 Mar 2025 20:51:28 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: WangYuli <wangyuli@...ontech.com>
Cc: kernel@...0n.name, guanwentao@...ontech.com, wentao@...ontech.com, 
	loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	zhoubinbin@...ngson.cn, lihui@...ngson.cn, rdunlap@...radead.org, 
	chenhuacai@...ngson.cn, zhanjun@...ontech.com, niecheng1@...ontech.com, 
	Tiezhu Yang <yangtiezhu@...ngson.cn>, Jinyang He <hejinyang@...ngson.cn>
Subject: Re: [PATCH v2] LoongArch: KGDB: Rework arch_kgdb_breakpoint() implementation

Hi, Tiezhu & Jinyang,

On Fri, Mar 21, 2025 at 10:11 PM WangYuli <wangyuli@...ontech.com> wrote:
>
> The arch_kgdb_breakpoint() function defines the kgdb_breakinst
> symbol using inline assembly.
>
> There's a potential issue where the compiler might inline
> arch_kgdb_breakpoint(), which would then define the kgdb_breakinst
> symbol multiple times, leading to a linker error.
>
> To prevent this, declare arch_kgdb_breakpoint() as noline.
>
> Fix follow error with LLVM-19 *only* when LTO_CLANG_FULL:
>     LD      vmlinux.o
>   ld.lld-19: error: ld-temp.o <inline asm>:3:1: symbol 'kgdb_breakinst' is already defined
>   kgdb_breakinst: break 2
>   ^
>
> Additionally, remove "nop" here because it's meaningless for LoongArch
> here.
>
> Fixes: e14dd076964e ("LoongArch: Add basic KGDB & KDB support")
> Co-developed-by: Winston Wen <wentao@...ontech.com>
> Signed-off-by: Winston Wen <wentao@...ontech.com>
> Co-developed-by: Wentao Guan <guanwentao@...ontech.com>
> Signed-off-by: Wentao Guan <guanwentao@...ontech.com>
> Co-developed-by: Huacai Chen <chenhuacai@...ngson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
> Tested-by: Yuli Wang <wangyuli@...ontech.com>
> Tested-by: Binbin Zhou <zhoubinbin@...ngson.cn>
> Signed-off-by: Yuli Wang <wangyuli@...ontech.com>
> ---
> Changelog:
>  *v1->v2:
>     1. Drop the nop which is no effect for LoongArch here.
>     2. Add "STACK_FRAME_NON_STANDARD" for arch_kgdb_breakpoint() to
> avoid the objtool warning.
> ---
>  arch/loongarch/kernel/kgdb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/kernel/kgdb.c b/arch/loongarch/kernel/kgdb.c
> index 445c452d72a7..38bd0561d7d5 100644
> --- a/arch/loongarch/kernel/kgdb.c
> +++ b/arch/loongarch/kernel/kgdb.c
> @@ -224,13 +224,13 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc)
>         regs->csr_era = pc;
>  }
>
> -void arch_kgdb_breakpoint(void)
> +noinline void arch_kgdb_breakpoint(void)
>  {
>         __asm__ __volatile__ (                  \
>                 ".globl kgdb_breakinst\n\t"     \
> -               "nop\n"                         \
>                 "kgdb_breakinst:\tbreak 2\n\t"); /* BRK_KDB = 2 */
>  }
> +STACK_FRAME_NON_STANDARD(arch_kgdb_breakpoint);
Is there a better solution than STACK_FRAME_NON_STANDARD()? In the
past we can use annotate_reachable() in arch_kgdb_breakpoint(), but
annotate_reachable() is no longer exist.

Huacai

>
>  /*
>   * Calls linux_debug_hook before the kernel dies. If KGDB is enabled,
> --
> 2.49.0
>