linux-kernel - Re: [PATCH] x86/fpu: Fix potential NULL dereference in avx512

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: <20250718071250.36019-1-wangfushuai@baidu.com>
Date: Fri, 18 Jul 2025 15:12:50 +0800
From: Fushuai Wang <wangfushuai@...du.com>
To: <sohil.mehta@...el.com>
CC: <aruna.ramakrishna@...cle.com>, <aubrey.li@...el.com>, <bp@...en8.de>,
	<brgerst@...il.com>, <chang.seok.bae@...el.com>,
	<dave.hansen@...ux.intel.com>, <hpa@...or.com>,
	<linux-kernel@...r.kernel.org>, <mingo@...hat.com>, <oleg@...hat.com>,
	<peterz@...radead.org>, <rick.p.edgecombe@...el.com>, <seanjc@...gle.com>,
	<tglx@...utronix.de>, <vigbalas@....com>, <wangfushuai@...du.com>,
	<x86@...nel.org>
Subject: Re: [PATCH] x86/fpu: Fix potential NULL dereference in avx512_status()

>> Can you please share any other warnings that were triggered before this
>> Oops message? Also, I'll try to generate this locally. Any specific
>> configuration needed for reproducing this apart from CONFIG_X86_DEBUG_FPU?
>
>I was able to reproduce this on a system with X86_FEATURE_AVX512F. The
>issue only happens while reading arch_status on a kthread.
>
>$cat /proc/[kthread]/arch_status     => NULL pointer exception
>$cat /proc/[user thread]/arch_status => No issue seen
>
>Can you confirm that you are seeing the same behavior?

Confirmed, same issue here.

>Unfortunately, avx512_timestamp resides within struct fpu. So getting
>that value for a kthread would mean going through x86_task_fpu().
>
>I am wondering if we ever need to expose the AVX512 usage for kernel
>threads? If not, then we can do what you currently have but without the
>CONFIG_X86_DEBUG_FPU restriction. All kernel threads would always print
>the AVX512_elapsed_ms as -1.
>
>However, this would be a user visible change so we should probably get
>more inputs. I tried this experiment on an older kernel without the
>above issue. Among all the active kthreads on my system a handful of
>them show a valid value for AVX512 usage. The rest of them all show -1.
>
>PID: 2594
>CMD: avahi-daemon: running [SAP.local]
>  /proc/2594/arch_status content:
>AVX512_elapsed_ms:      46032
>
>PID: 2729
>CMD: sshd: /usr/sbin/sshd -D [listener] 0 of 10-100 startups
>  /proc/2729/arch_status content:
>AVX512_elapsed_ms:      396656
>
>To keep the older behavior, we might need to consider moving
>avx512_timestamp out of struct fpu. Though, I am uncertain about its
>implication.

I think avx512_elapsed_ms should logically belong in the FPU structure,
as it's a field inherently tied to FPU operations? To keep the older
behavior, we can set CONFIG_X86_DEBUG_FPU=n, maybe?
Is there a better approach to ensure kernel threads always correctly output
avx512_elapsed_ms. Directly get FPU struct pointer without using x86_task_fpu()?

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 9aa9ac8399ae..f989bc125e9b 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1859,9 +1859,10 @@ long fpu_xstate_prctl(int option, unsigned long arg2)
  */
 static void avx512_status(struct seq_file *m, struct task_struct *task)
 {
-       unsigned long timestamp = READ_ONCE(x86_task_fpu(task)->avx512_timestamp);
+       unsigned long timestamp;
        long delta;

+       timestamp = READ_ONCE((struct fpu *)((void *)(task) + sizeof(*(task)))->avx512_timestamp);
        if (!timestamp) {
                /*
                 * Report -1 if no AVX512 usage