| 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: <CAAhV-H5E79jZ9rMYyP3h+KLX5rDGcB-Ec8WqyAhPC-pvuvv5UQ@mail.gmail.com>
Date: Wed, 5 Nov 2025 08:57:39 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Bill Tsui <b10902118@....edu.tw>
Cc: oleg@...hat.com, kernel@...0n.name, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] LoongArch: ptrace: Merge ptrace_hbp_set_*()
Hi, Tiezhu,
On Wed, Oct 29, 2025 at 11:38 PM Bill Tsui <b10902118@....edu.tw> wrote:
>
> In hw_break_set(), those functions actually can be combined into one.
> This eliminates intermediate calls to modify_user_hw_breakpoint() that
> may leave hardware registers in a partially updated state.
>
> This redundancy was originally found in ARM ptrace, where it caused
> non-4-byte address alignment checks to fail.
>
> Link:
> https://lore.kernel.org/all/20251018133731.42505-2-b10902118@ntu.edu.tw/
>
> The LoongArch implementation appears to have been derived from ARM,
> so this refactor helps avoid the same issue and simplifies future
> maintenance.
What do you think about it?
Huacai
>
> Signed-off-by: Bill Tsui <b10902118@....edu.tw>
> ---
> arch/loongarch/kernel/ptrace.c | 69 +++++++++-------------------------
> 1 file changed, 17 insertions(+), 52 deletions(-)
>
> diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c
> index 8edd0954e55a..c6cd51569a64 100644
> --- a/arch/loongarch/kernel/ptrace.c
> +++ b/arch/loongarch/kernel/ptrace.c
> @@ -581,13 +581,15 @@ static int ptrace_hbp_get_addr(unsigned int note_type,
> return 0;
> }
>
> -static int ptrace_hbp_set_ctrl(unsigned int note_type,
> +static int ptrace_hbp_set(unsigned int note_type,
> struct task_struct *tsk,
> - unsigned long idx, u32 uctrl)
> + unsigned long idx, u64 addr,
> + u64 mask, u32 uctrl)
> {
> int err;
> struct perf_event *bp;
> struct perf_event_attr attr;
> + struct arch_hw_breakpoint *info;
> struct arch_hw_breakpoint_ctrl ctrl;
> struct thread_info *ti = task_thread_info(tsk);
>
> @@ -597,6 +599,17 @@ static int ptrace_hbp_set_ctrl(unsigned int note_type,
>
> attr = bp->attr;
>
> + /* addr */
> + /* Kernel-space address cannot be monitored by user-space */
> + if ((unsigned long)addr >= XKPRANGE)
> + return -EINVAL;
> + attr.bp_addr = addr;
> +
> + /* mask */
> + info = counter_arch_bp(bp);
> + info->mask = mask;
> +
> + /* ctrl */
> switch (note_type) {
> case NT_LOONGARCH_HW_BREAK:
> ctrl.type = LOONGARCH_BREAKPOINT_EXECUTE;
> @@ -623,46 +636,6 @@ static int ptrace_hbp_set_ctrl(unsigned int note_type,
> return modify_user_hw_breakpoint(bp, &attr);
> }
>
> -static int ptrace_hbp_set_mask(unsigned int note_type,
> - struct task_struct *tsk,
> - unsigned long idx, u64 mask)
> -{
> - struct perf_event *bp;
> - struct perf_event_attr attr;
> - struct arch_hw_breakpoint *info;
> -
> - bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
> - if (IS_ERR(bp))
> - return PTR_ERR(bp);
> -
> - attr = bp->attr;
> - info = counter_arch_bp(bp);
> - info->mask = mask;
> -
> - return modify_user_hw_breakpoint(bp, &attr);
> -}
> -
> -static int ptrace_hbp_set_addr(unsigned int note_type,
> - struct task_struct *tsk,
> - unsigned long idx, u64 addr)
> -{
> - struct perf_event *bp;
> - struct perf_event_attr attr;
> -
> - /* Kernel-space address cannot be monitored by user-space */
> - if ((unsigned long)addr >= XKPRANGE)
> - return -EINVAL;
> -
> - bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
> - if (IS_ERR(bp))
> - return PTR_ERR(bp);
> -
> - attr = bp->attr;
> - attr.bp_addr = addr;
> -
> - return modify_user_hw_breakpoint(bp, &attr);
> -}
> -
> #define PTRACE_HBP_ADDR_SZ sizeof(u64)
> #define PTRACE_HBP_MASK_SZ sizeof(u64)
> #define PTRACE_HBP_CTRL_SZ sizeof(u32)
> @@ -733,10 +706,6 @@ static int hw_break_set(struct task_struct *target,
> offset, offset + PTRACE_HBP_ADDR_SZ);
> if (ret)
> return ret;
> -
> - ret = ptrace_hbp_set_addr(note_type, target, idx, addr);
> - if (ret)
> - return ret;
> offset += PTRACE_HBP_ADDR_SZ;
>
> if (!count)
> @@ -746,21 +715,17 @@ static int hw_break_set(struct task_struct *target,
> offset, offset + PTRACE_HBP_MASK_SZ);
> if (ret)
> return ret;
> -
> - ret = ptrace_hbp_set_mask(note_type, target, idx, mask);
> - if (ret)
> - return ret;
> offset += PTRACE_HBP_MASK_SZ;
>
> ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &ctrl,
> offset, offset + PTRACE_HBP_CTRL_SZ);
> if (ret)
> return ret;
> + offset += PTRACE_HBP_CTRL_SZ;
>
> - ret = ptrace_hbp_set_ctrl(note_type, target, idx, ctrl);
> + ret = ptrace_hbp_set(note_type, target, idx, addr, mask, ctrl);
> if (ret)
> return ret;
> - offset += PTRACE_HBP_CTRL_SZ;
>
> user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
> offset, offset + PTRACE_HBP_PAD_SZ);
> --
> 2.51.0
>
Powered by blists - more mailing lists