Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH v4 0/2] x86/fpu: Fix dynamic fpstate leak on exec()
@ 2026-11-02 10:08 Guixiong Wei
  2026-11-02 10:30 ` [PATCH v4 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec() Guixiong Wei
  0 siblings, 1 reply; 4+ messages in thread
From: Guixiong Wei @ 2026-11-02 10:08 UTC (permalink / raw)
  To: x86
  Cc: Guixiong Wei, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H . Peter Anvin, Chang S . Bae, Shuah Khan,
	linux-kernel, linux-kselftest, stable

An exec() after using an XFD-controlled xfeature leaks the dynamically
allocated fpstate. Fix the cleanup in fpstate_reset() and add an AMX
regression selftest.

The selftest reports FAIL on an unfixed kernel (10/10 FAILs on 7.1.3),
SKIP when vmallocinfo addresses are hidden, and OK with the fix applied.

Changes since v3:
- Close the use-after-free that freeing in fpstate_reset() exposed.
  fpu_flush_thread() called fpstate_reset() before
  fpu_reset_fpstate_regs() set TIF_NEED_FPU_LOAD, so a context switch
  or a kernel FPU user in that window could still see live register
  state and save it into the freed buffer. Set TIF_NEED_FPU_LOAD before
  the reset and document on fpstate_reset() that callers operating on
  the current task must prevent such a save. The race was caught by
  the Sashiko review and the fix was suggested by Chang S. Bae.

v3: https://lore.kernel.org/r/cover.1793072271.git.weiguixiong@bytedance.com
v2: https://lore.kernel.org/r/cover.1791428412.git.weiguixiong@bytedance.com
v1: https://lore.kernel.org/r/20260929151013.81562-2-weiguixiong@bytedance.com

Guixiong Wei (2):
  x86/fpu: Fix memory leak with dynamic fpstate and exec()
  selftests/x86/amx: Test dynamic fpstate cleanup across exec()

 arch/x86/kernel/fpu/core.c        |  12 ++
 tools/testing/selftests/x86/amx.c | 251 +++++++++++++++++++++++++++++-
 2 files changed, 261 insertions(+), 2 deletions(-)


base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.55.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec()
  2026-11-02 10:08 [PATCH v4 0/2] x86/fpu: Fix dynamic fpstate leak on exec() Guixiong Wei
@ 2026-11-02 10:30 ` Guixiong Wei
  2026-11-06 16:40   ` Chang S. Bae
  0 siblings, 1 reply; 4+ messages in thread
From: Guixiong Wei @ 2026-11-02 10:30 UTC (permalink / raw)
  To: x86
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chang S . Bae, Shuah Khan, linux-kernel,
	linux-kselftest

Add a regression test for dynamic fpstate leaks across exec(). Repeatedly
fork a child, load XTILEDATA to allocate dynamic state, and exec the test
binary. Snapshot the start addresses of matching /proc/vmallocinfo entries
before and after the loop; addresses present only in the "after" snapshot
count as new allocations that outlived their task.

Before the first exec(), verify that the child's allocation is visible.
Skip the check if vmallocinfo is inaccessible, if a matching entry's
address is hidden by kptr_restrict, or if the allocation cannot be
observed; address-set diff cannot distinguish allocations in those cases.

The test assumes no concurrent workload creates matching allocations.
A set difference equal to the iteration count indicates a leak; an empty
difference indicates success. Report other counts as inconclusive.

Assisted-by: LLM
Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
---
 tools/testing/selftests/x86/amx.c | 251 +++++++++++++++++++++++++++++-
 1 file changed, 249 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c
index 40769c16de1bb..ea35d5332dc3f 100644
--- a/tools/testing/selftests/x86/amx.c
+++ b/tools/testing/selftests/x86/amx.c
@@ -5,6 +5,7 @@
 #include <errno.h>
 #include <setjmp.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <stdbool.h>
 #include <unistd.h>
@@ -30,6 +31,9 @@
 #define XFEATURE_MASK_XTILEDATA	(1 << XFEATURE_XTILEDATA)
 #define XFEATURE_MASK_XTILE	(XFEATURE_MASK_XTILECFG | XFEATURE_MASK_XTILEDATA)
 
+#define EXEC_TEST_ARG		"--exec-test-child"
+#define EXEC_TEST_ITERS		10
+
 struct xstate_info xtiledata;
 
 /* The helpers for managing XSAVE buffer and tile states: */
@@ -478,11 +482,253 @@ static void test_fork(void)
 	_exit(0);
 }
 
-int main(void)
+/*
+ * Snapshot start addresses of /proc/vmallocinfo entries whose caller
+ * matches a dynamic fpstate allocation. Returns false if the file is
+ * not readable (default mode 0400), or if a matching entry's address
+ * cannot be used (kptr_restrict hides the address, or parsing fails);
+ * in those cases the caller cannot distinguish allocations and must
+ * SKIP rather than report a potentially false PASS.
+ */
+struct fpstate_snapshot {
+	unsigned long *addrs;
+	size_t len, cap;
+};
+
+static int cmp_ulong(const void *a, const void *b)
+{
+	unsigned long x = *(const unsigned long *)a, y = *(const unsigned long *)b;
+
+	return (x > y) - (x < y);
+}
+
+static bool snapshot_fpstates(struct fpstate_snapshot *s)
+{
+	char *line = NULL;
+	size_t line_size = 0;
+	bool ok = true;
+	FILE *fp;
+
+	s->addrs = NULL;
+	s->len = 0;
+	s->cap = 0;
+
+	fp = fopen("/proc/vmallocinfo", "r");
+	if (!fp) {
+		if (errno == EACCES || errno == EPERM || errno == ENOENT)
+			return false;
+		fatal_error("fopen /proc/vmallocinfo");
+	}
+
+	while (getline(&line, &line_size, fp) >= 0) {
+		unsigned long addr;
+
+		/*
+		 * fpstate_realloc() is static and may or may not be inlined
+		 * into __xfd_enable_feature() depending on build options;
+		 * match either caller to stay compiler-independent.
+		 */
+		if (!strstr(line, "__xfd_enable_feature") &&
+		    !strstr(line, "fpstate_realloc"))
+			continue;
+		/*
+		 * vmallocinfo prints the start address with %pK. If
+		 * kptr_restrict hides it (address shown as 0) or the
+		 * format is unexpected, address-set diff loses the
+		 * ability to distinguish allocations. Bail out so the
+		 * caller SKIPs instead of silently masking a leak.
+		 */
+		if (sscanf(line, "0x%lx", &addr) != 1 || !addr) {
+			ok = false;
+			break;
+		}
+		if (s->len == s->cap) {
+			unsigned long *tmp;
+
+			s->cap = s->cap ? s->cap * 2 : 64;
+			tmp = realloc(s->addrs, s->cap * sizeof(*s->addrs));
+			if (!tmp)
+				fatal_error("realloc snapshot");
+			s->addrs = tmp;
+		}
+		s->addrs[s->len++] = addr;
+	}
+
+	if (ferror(fp))
+		fatal_error("read /proc/vmallocinfo");
+
+	free(line);
+	fclose(fp);
+	if (!ok) {
+		free(s->addrs);
+		s->addrs = NULL;
+		s->len = 0;
+		s->cap = 0;
+		return false;
+	}
+	qsort(s->addrs, s->len, sizeof(*s->addrs), cmp_ulong);
+	return true;
+}
+
+static int count_new(const struct fpstate_snapshot *before,
+		     const struct fpstate_snapshot *after)
+{
+	int n = 0;
+	size_t i;
+
+	for (i = 0; i < after->len; i++)
+		if (!bsearch(&after->addrs[i], before->addrs, before->len,
+			     sizeof(*before->addrs), cmp_ulong))
+			n++;
+	return n;
+}
+
+static int exec_test_fails;
+
+static void check_exec_child_status(int status)
+{
+	if (WIFEXITED(status) && !WEXITSTATUS(status))
+		return;
+	if (WIFSIGNALED(status))
+		errx(1, "[FAIL]\texec test child killed by signal %d",
+		     WTERMSIG(status));
+	errx(1, "[FAIL]\texec test child exited with status %d",
+	     WEXITSTATUS(status));
+}
+
+/*
+ * Fork a child that loads tiledata and blocks on a pipe before exec().
+ * The parent samples /proc/vmallocinfo while the child is still suspended
+ * and verifies that the dynamic fpstate allocation is observable. Without
+ * that visibility the test cannot distinguish "released" from "never
+ * observed", so it reports SKIP instead of a potentially false PASS.
+ */
+static bool observe_dynamic_fpstate(const struct fpstate_snapshot *before)
+{
+	struct fpstate_snapshot mid;
+	int ready_pipe[2], go_pipe[2];
+	pid_t child;
+	int status;
+	bool observable;
+	char c = 0;
+
+	if (pipe(ready_pipe) || pipe(go_pipe))
+		fatal_error("pipe");
+
+	child = fork();
+	if (child < 0)
+		fatal_error("fork");
+	if (!child) {
+		close(ready_pipe[0]);
+		close(go_pipe[1]);
+		if (!load_rand_tiledata(stashed_xsave))
+			fatal_error("failed to load tiledata");
+		if (write(ready_pipe[1], &c, 1) != 1)
+			fatal_error("write ready");
+		if (read(go_pipe[0], &c, 1) != 1)
+			fatal_error("read go");
+		close(ready_pipe[1]);
+		close(go_pipe[0]);
+		execl("/proc/self/exe", "amx", EXEC_TEST_ARG, NULL);
+		fatal_error("exec");
+	}
+
+	close(ready_pipe[1]);
+	close(go_pipe[0]);
+
+	if (read(ready_pipe[0], &c, 1) != 1)
+		fatal_error("read ready");
+
+	observable = snapshot_fpstates(&mid) && count_new(before, &mid) > 0;
+	free(mid.addrs);
+
+	if (write(go_pipe[1], &c, 1) != 1)
+		fatal_error("write go");
+
+	close(ready_pipe[0]);
+	close(go_pipe[1]);
+
+	if (waitpid(child, &status, 0) != child)
+		fatal_error("waitpid");
+	check_exec_child_status(status);
+
+	return observable;
+}
+
+static void test_exec(void)
+{
+	struct fpstate_snapshot before, after;
+	int new_count, i;
+
+	printf("[RUN]\tCheck dynamic fpstate cleanup across exec().\n");
+
+	if (!snapshot_fpstates(&before)) {
+		printf("[SKIP]\tvmallocinfo inaccessible or addresses hidden by kptr_restrict.\n");
+		return;
+	}
+
+	if (!observe_dynamic_fpstate(&before)) {
+		free(before.addrs);
+		printf("[SKIP]\tCannot observe dynamic fpstate allocations via vmallocinfo.\n");
+		return;
+	}
+	free(before.addrs);
+
+	if (!snapshot_fpstates(&before))
+		fatal_error("snapshot before");
+
+	for (i = 0; i < EXEC_TEST_ITERS; i++) {
+		pid_t child;
+		int status;
+
+		child = fork();
+		if (child < 0)
+			fatal_error("fork");
+		if (!child) {
+			if (!load_rand_tiledata(stashed_xsave))
+				fatal_error("failed to load tiledata");
+			execl("/proc/self/exe", "amx", EXEC_TEST_ARG, NULL);
+			fatal_error("exec");
+		}
+
+		if (waitpid(child, &status, 0) != child)
+			fatal_error("waitpid");
+		check_exec_child_status(status);
+	}
+
+	if (!snapshot_fpstates(&after)) {
+		free(before.addrs);
+		printf("[SKIP]\tvmallocinfo inaccessible or addresses hidden by kptr_restrict.\n");
+		return;
+	}
+	new_count = count_new(&before, &after);
+	free(before.addrs);
+	free(after.addrs);
+
+	if (!new_count) {
+		printf("[OK]\tDynamic fpstate allocations were freed across exec().\n");
+		return;
+	}
+
+	if (new_count == EXEC_TEST_ITERS) {
+		printf("[FAIL]\texec() leaked %d dynamic fpstate allocations\n",
+		       new_count);
+		exec_test_fails++;
+		return;
+	}
+
+	printf("[SKIP]\tUnexpected new allocation count %d; result inconclusive.\n",
+	       new_count);
+}
+
+int main(int argc, char **argv)
 {
 	unsigned long features;
 	long rc;
 
+	if (argc == 2 && !strcmp(argv[1], EXEC_TEST_ARG))
+		return 0;
+
 	rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_SUPP, &features);
 	if (rc || (features & XFEATURE_MASK_XTILE) != XFEATURE_MASK_XTILE) {
 		ksft_print_msg("no AMX support\n");
@@ -504,6 +750,7 @@ int main(void)
 	req_xtiledata_perm();
 
 	test_fork();
+	test_exec();
 
 	/*
 	 * Perform generic xstate tests for context switching, ptrace,
@@ -514,5 +761,5 @@ int main(void)
 	clearhandler(SIGILL);
 	free_stashed_xsave();
 
-	return 0;
+	return exec_test_fails ? 1 : 0;
 }
-- 
2.55.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec()
  2026-11-02 10:30 ` [PATCH v4 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec() Guixiong Wei
@ 2026-11-06 16:40   ` Chang S. Bae
  2026-11-08 13:19     ` Guixiong Wei
  0 siblings, 1 reply; 4+ messages in thread
From: Chang S. Bae @ 2026-11-06 16:40 UTC (permalink / raw)
  To: Guixiong Wei, x86
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chang S . Bae, Shuah Khan, linux-kernel,
	linux-kselftest

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

On 11/2/26 02:30, Guixiong Wei wrote:
> ---
>   tools/testing/selftests/x86/amx.c | 251 +++++++++++++++++++++++++++++-
>   1 file changed, 249 insertions(+), 2 deletions(-)
> 

The code looks an overkill to me. I also considered using kmemleak, but 
this vmallocinfo looks to be efficient.

The other way would be adding a script on top of a small change to amx.c 
file. See the attachment.

Can you please try running this on your side and see if it works?

PS: I sent this yesterday but it didn't go through. I switched *.sh to 
*.txt in the attachment.

Thanks,
Chang

[-- Attachment #2: amx_exec.txt --]
[-- Type: text/plain, Size: 1500 bytes --]

#!/usr/bin/env bash

NUM_ITERATION=10

if [ "$EUID" -ne 0 ]; then
    echo "Error: Please run as root (sudo) to access /proc/vmallocinfo."
    exit 1
fi

TMP_BEFORE=$(mktemp)
TMP_AFTER=$(mktemp)

# Function to parse and aggregate vmallocinfo by caller/module
parse_vmalloc() {
    # Typical format: 0xffffc90000000000-0xffffc90000002000    8192 pcpu_alloc_alloc+0x3b/0x240 vmalloc
    # We aggregate the size (column 2) grouped by the caller function (column 3)
    awk '
    {
        # Remove offset details like +0x3b/0x240 to group by base function name
        split($3, caller, "+")
        # Keep track of caller type if present (e.g. vmalloc, ioremap)
        type = $4 ? " (" $4 ")" : ""
        key = caller[1] type
        sum[key] += $2
    }
    END {
        for (k in sum) {
            print k "," sum[k]
        }
    }' /proc/vmallocinfo
}

parse_vmalloc > "$TMP_BEFORE"
./amx_64 --exec-test $NUM_ITERATION
parse_vmalloc > "$TMP_AFTER"


# Join both files on the caller name and compute the differences
LEAKED=$(awk -F, '
NR == FNR {
    before[$1] = $2
    next
}
{
    after[$1] = $2
    diff = $2 - before[$1]
    if ($1 ~ /__xfd_enable_feature/ && diff > 1024*4*3*$NUM_ITERATION) {
        found=1
        #print "%-40s %+15d\n", $1, diff
    }
    delete before[$1] # Mark as processed
} END {
    print found+0
}' "$TMP_BEFORE" "$TMP_AFTER")

if [ "$LEAKED" == "1" ]; then
    echo "[FAIL] memory leak"
else
    echo "[OK] no memory leak"
fi

rm -f "$TMP_BEFORE" "$TMP_AFTER"

[-- Attachment #3: temp.diff --]
[-- Type: text/x-patch, Size: 1890 bytes --]

diff --git a/tools/testing/selftests/x86/amx.c b/tools/testing/selftests/x86/amx.c
index 40769c16de1b..58bbeac3a023 100644
--- a/tools/testing/selftests/x86/amx.c
+++ b/tools/testing/selftests/x86/amx.c
@@ -5,6 +5,7 @@
 #include <errno.h>
 #include <setjmp.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <stdbool.h>
 #include <unistd.h>
@@ -30,6 +31,9 @@
 #define XFEATURE_MASK_XTILEDATA	(1 << XFEATURE_XTILEDATA)
 #define XFEATURE_MASK_XTILE	(XFEATURE_MASK_XTILECFG | XFEATURE_MASK_XTILEDATA)
 
+#define EXEC_TEST_ARG		"--exec-test"
+#define EXEC_TEST_CHILD_ARG	"--exec-test-child"
+
 struct xstate_info xtiledata;
 
 /* The helpers for managing XSAVE buffer and tile states: */
@@ -478,11 +482,44 @@ static void test_fork(void)
 	_exit(0);
 }
 
-int main(void)
+static int test_exec(int argc, char **argv)
+{
+	int i, times = atoi(argv[2]);
+
+	req_xtiledata_perm();
+
+	for (i = 0; i < times; i++) {
+		pid_t child;
+		int status;
+
+		child = fork();
+		if (child < 0)
+			fatal_error("fork");
+		if (!child) {
+			char *args[] = {argv[0], EXEC_TEST_CHILD_ARG, NULL};
+
+			if (!load_rand_tiledata(stashed_xsave))
+				fatal_error("failed to load tiledata");
+
+			execv(args[0], args);
+			fatal_error("exec");
+		}
+
+		if (waitpid(child, &status, 0) != child)
+			fatal_error("waitpid");
+	}
+
+	return 0;
+}
+
+int main(int argc, char **argv)
 {
 	unsigned long features;
 	long rc;
 
+	if (argc == 2 && !strcmp(argv[1], EXEC_TEST_CHILD_ARG))
+			return 0;
+
 	rc = syscall(SYS_arch_prctl, ARCH_GET_XCOMP_SUPP, &features);
 	if (rc || (features & XFEATURE_MASK_XTILE) != XFEATURE_MASK_XTILE) {
 		ksft_print_msg("no AMX support\n");
@@ -496,6 +533,10 @@ int main(void)
 	}
 
 	init_stashed_xsave();
+
+	if (argc == 3 && !strcmp(argv[1], EXEC_TEST_ARG))
+		return test_exec(argc, argv);
+
 	sethandler(SIGILL, handle_noperm, 0);
 
 	test_dynamic_state();

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec()
  2026-11-06 16:40   ` Chang S. Bae
@ 2026-11-08 13:19     ` Guixiong Wei
  0 siblings, 0 replies; 4+ messages in thread
From: Guixiong Wei @ 2026-11-08 13:19 UTC (permalink / raw)
  To: Chang S. Bae
  Cc: x86, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	H . Peter Anvin, Chang S . Bae, Shuah Khan, linux-kernel,
	linux-kselftest, Guixiong Wei


> From: "Chang S. Bae"<chang.seok.bae@intel.com>
> Date:  Sat, Nov 7, 2026, 00:41
> Subject:  Re: [PATCH v4 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec()
> To: "Guixiong Wei"<weiguixiong@bytedance.com>, <x86@kernel.org>
> Cc: "Thomas Gleixner"<tglx@kernel.org>, "Ingo Molnar"<mingo@redhat.com>, "Borislav Petkov"<bp@alien8.de>, "Dave Hansen"<dave.hansen@linux.intel.com>, "H . Peter Anvin"<hpa@zytor.com>, "Chang S . Bae"<chang.seok.bae@intel.com>, "Shuah Khan"<shuah@kernel.org>, <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>
> On 11/2/26 02:30, Guixiong Wei wrote:
> > ---
> >   tools/testing/selftests/x86/amx.c | 251 +++++++++++++++++++++++++++++-
> >   1 file changed, 249 insertions(+), 2 deletions(-)
> > 
> 
> The code looks an overkill to me. I also considered using kmemleak, but 
> this vmallocinfo looks to be efficient.
> 
> The other way would be adding a script on top of a small change to amx.c 
> file. See the attachment.
> 
> Can you please try running this on your side and see if it works?
> 
> PS: I sent this yesterday but it didn't go through. I switched *.sh to 
> *.txt in the attachment.
> 
> Thanks,
> Chang
> 
Hi Chang,

Thanks for the suggestion and the test patch. The AMX -> exec reproducer
does exercise the leaking path and was useful for validating the fix.

I tested it on an Intel Xeon 6519P-C system with AMX support.

On the unfixed kernel, three runs of 10 execs each consistently
increased the __xfd_enable_feature allocations by 10 entries / 163840
bytes per run.

With the updated fix applied to v7.3-rc5:
  - three runs of 10 execs: no increase
  - one run of 1000 execs: no increase
  - 24 concurrent workers, 500 execs each: no increase
  - the existing AMX selftest still passed

I noticed a few issues in the test harness itself:
  1. test_exec() waits for the child but does not check WIFEXITED() or
     WEXITSTATUS(), so a failed exec can still be reported as success.
  2. $NUM_ITERATION in the single-quoted awk program is interpreted as
     an awk field reference, not the shell variable. It needs to be
     passed with -v, for example:
       awk -v iterations="$NUM_ITERATION" '... iterations ...'
  3. amx_exec.sh prints "[FAIL]" for a detected leak but still exits
     with status 0.
  4. Failures while collecting /proc/vmallocinfo or running amx_64 can
     be converted into "[OK] no memory leak".

So the reproducer idea looks good to me, while the harness needs the above fixes.

Thanks,
Guixiong

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-08 13:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 10:08 [PATCH v4 0/2] x86/fpu: Fix dynamic fpstate leak on exec() Guixiong Wei
2026-11-02 10:30 ` [PATCH v4 2/2] selftests/x86/amx: Test dynamic fpstate cleanup across exec() Guixiong Wei
2026-11-06 16:40   ` Chang S. Bae
2026-11-08 13:19     ` Guixiong Wei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox