linux-kernel - Re: [PATCH] Loongarch:Prevent cond_resched occurring within kernel-fpu

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] [day] [month] [year] [list]

Message-ID: <CAAhV-H5+kb4xiiSanFpx=xfyOuCxi5AdyF5cJ8rV9vm7UEBJPg@mail.gmail.com>
Date: Wed, 7 May 2025 22:03:47 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Tianyang Zhang <zhangtianyang@...ngson.cn>
Cc: kernel@...0n.name, bigeasy@...utronix.de, clrkwllms@...nel.org, 
	rostedt@...dmis.org, loongarch@...ts.linux.dev, 
	linux-rt-devel@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Loongarch:Prevent cond_resched occurring within kernel-fpu

Hi, Tianyang,

LoongArch is not Loongarch in the title.

On Wed, May 7, 2025 at 6:06 PM Tianyang Zhang <zhangtianyang@...ngson.cn> wrote:
>
> When CONFIG_PREEMPT_COUNT is not configured, preempt_disable/preempt_enable
> merely acts as a barrier. However, cond_resched can still trigger a
> context switch and modify the EUEN, resulting in do_fpu being activated
> within kernel-fpu critical section, as demonstrated in the following path:
>
> dcn32_calculate_wm_and_dlg
>     DC_FP_START
>         dcn32_calculate_wm_and_dlg_fpu
>             dcn32_find_dummy_latency_index_for_fw_based_mclk_switch
>                 dcn32_internal_validate_bw
>                     dcn32_enable_phantom_stream
>                         dc_create_stream_for_sink
>                            kzalloc(GFP_KERNEL)
>                                 __kmem_cache_alloc_node
>                                     __cond_resched
>     DC_FP_END
>
> This patch is similar to d02198550423a0b695e7a24ec77153209ad45b09
> (x86/fpu: Improve crypto performance by making kernel-mode FPU reliably
> usablein softirqs),to avoids the issue, and extends kernel-fpu application
> scenarios to softirq.
>

You need to Cc the stable list here.

> Signed-off-by: Tianyang Zhang <zhangtianyang@...ngson.cn>
> ---
>  arch/loongarch/kernel/kfpu.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/arch/loongarch/kernel/kfpu.c b/arch/loongarch/kernel/kfpu.c
> index ec5b28e570c9..e60bbaca357a 100644
> --- a/arch/loongarch/kernel/kfpu.c
> +++ b/arch/loongarch/kernel/kfpu.c
> @@ -18,11 +18,28 @@ static unsigned int euen_mask = CSR_EUEN_FPEN;
>  static DEFINE_PER_CPU(bool, in_kernel_fpu);
>  static DEFINE_PER_CPU(unsigned int, euen_current);
>
> +static inline void fpu_lock(void)
The x86 naming, fpregs_lock() is better than fpu_lock(), we can keep the same.

To save time, I have fixed them [1], you can fetch it to send V2.
[1] https://github.com/chenhuacai/linux/commit/fa0952bb7667b138a8941bb3e86775adc9f4731f

Huacai

> +{
> +       if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> +               local_bh_disable();
> +       else
> +               preempt_disable();
> +}
> +
> +static inline void fpu_unlock(void)
> +{
> +       if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> +               local_bh_enable();
> +       else
> +               preempt_enable();
> +}
> +
>  void kernel_fpu_begin(void)
>  {
>         unsigned int *euen_curr;
>
> -       preempt_disable();
> +       if (!irqs_disabled())
> +               fpu_lock();
>
>         WARN_ON(this_cpu_read(in_kernel_fpu));
>
> @@ -73,7 +90,8 @@ void kernel_fpu_end(void)
>
>         this_cpu_write(in_kernel_fpu, false);
>
> -       preempt_enable();
> +       if (!irqs_disabled())
> +               fpu_unlock();
>  }
>  EXPORT_SYMBOL_GPL(kernel_fpu_end);
>
> --
> 2.20.1
>