Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds
@ 2026-05-21  7:24 Ian Rogers
  2026-05-21  7:24 ` [PATCH v1 1/3] perf build: Unconditionally set up libunwind feature build flags Ian Rogers
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Ian Rogers @ 2026-05-21  7:24 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	linux-kernel, linux-perf-users

The combination of the libunwind refactoring and build system changes
exposed a build-test regression for the libunwind build using a cached
feature dump. Re-send kwork fixes [1] moving a definition to kwork.h
to avoid breaking no libbpf builds.

1. Build Fix for libunwind:
   "make feature-dump" runs without LIBUNWIND=1, skipping the setup of
   libunwind LDFLAGS. When Makefile.feature is included to generate the
   cached FEATURE-DUMP, the local test-libunwind.bin compiles without
   arch-specific link flags (-lunwind-x86_64) and fails, caching a false
   negative ("feature-libunwind=0"). Later builds (such as make_libunwind_O
   reusing the dump) compile the unwinder based on CONFIG_LIBUNWIND (remote)
   but disable HAVE_LIBUNWIND_SUPPORT (local), breaking compilation due to
   missing maps__e_machine declarations in maps.h. Fixed by unconditionally
   setting up libunwind feature build flags in Makefile.config.

2. kwork ASAN Double-Free & Leaks:
   - Solves double-free in the record command argument array due to
     parse_options mutation, by preserving original pointers in a calloc'ed
     to_free buffer.
   - Ensures kwork_usage string is freed on all exit paths.

3. kwork Unified Memory Lifecycle:
   - Establishes a unified memory ownership model for kwork_work by
     ensuring work names are dynamically duplicated via strdup() and
     consistently freed using work_exit() and a central perf_kwork__exit()
     teardown path.

[1] https://lore.kernel.org/linux-perf-users/20260520190538.142018-32-irogers@google.com/

Ian Rogers (3):
  perf build: Unconditionally set up libunwind feature build flags
  perf kwork: Fix address sanitizer issues
  perf kwork: Fix memory management of kwork_work

 tools/perf/Makefile.config  | 103 ++++++++++++------------
 tools/perf/builtin-kwork.c  | 151 ++++++++++++++++++++++++++++--------
 tools/perf/util/bpf_kwork.c |  14 +++-
 tools/perf/util/kwork.h     |   9 +++
 4 files changed, 188 insertions(+), 89 deletions(-)

-- 
2.54.0.746.g67dd491aae-goog


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

* [PATCH v1 1/3] perf build: Unconditionally set up libunwind feature build flags
  2026-05-21  7:24 [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
@ 2026-05-21  7:24 ` Ian Rogers
  2026-05-21  7:24 ` [PATCH v1 2/3] perf kwork: Fix address sanitizer issues Ian Rogers
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Ian Rogers @ 2026-05-21  7:24 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	linux-kernel, linux-perf-users

A "make feature-dump" build does not specify LIBUNWIND=1 because it is
run with the default configuration to detect system-wide capabilities.
This sets NO_LIBUNWIND := 1, causing Makefile.config to skip setting
LIBUNWIND_LIBS and FEATURE_CHECK_LDFLAGS-libunwind.

Consequently, when Makefile.feature is included and attempts to run
all feature checks (via FEATURE_TESTS := all), the local feature test
test-libunwind.bin compiles without the required architecture-specific
library flags (-lunwind-x86_64) and fails to link on x86_64. This
results in a corrupted cached BUILD_TEST_FEATURE_DUMP showing
feature-libunwind=0 even when the host supports it.

Subsequent test builds (like make_libunwind_O in the build-test suite)
which reuse the feature dump and specify LIBUNWIND=1 will fail to
compile due to a mismatch where CONFIG_LIBUNWIND is set (via remote
architecture checks which are self-contained) but HAVE_LIBUNWIND_SUPPORT
is disabled, causing compiler errors due to missing maps__e_machine
definitions in maps.h.

Fix this by unconditionally setting up the libunwind library lists
and feature check LDFLAGS in Makefile.config so they are always
populated and available to the feature detection engine regardless
of whether LIBUNWIND=1 is opted-in for the current run.

Fixes: 444508cd7c7b ("perf build: Be more programmatic when setting up libunwind variables")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Makefile.config | 103 +++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 51 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index b56fa8419f7d..c531b9315609 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -79,42 +79,43 @@ ifeq ($(ARCH),s390)
   CFLAGS += -fPIC
 endif
 
+# Unconditionally set up the libunwind feature build flags as a
+# feature-dump build doesn't specify LIBUNWIND=1. This means that
+# dumping the libunwind features will be broken that can impact later
+# builds that use the feature dump.
+ifeq ($(SRCARCH),arm)
+  LIBUNWIND_LIBS = -lunwind -lunwind-arm
+endif
+ifeq ($(SRCARCH),arm64)
+  LIBUNWIND_LIBS = -lunwind -lunwind-aarch64
+endif
+ifeq ($(SRCARCH),loongarch)
+  LIBUNWIND_LIBS = -lunwind -lunwind-loongarch64
+endif
+ifeq ($(ARCH),mips)
+  LIBUNWIND_LIBS = -lunwind -lunwind-mips
+endif
+ifeq ($(SRCARCH),powerpc)
+  LIBUNWIND_LIBS := -lunwind -lunwind-ppc64
+endif
+ifeq ($(SRCARCH),riscv)
+  LIBUNWIND_LIBS := -lunwind -lunwind-riscv
+endif
+ifeq ($(SRCARCH),s390)
+  LIBUNWIND_LIBS := -lunwind -lunwind-s390x
+endif
+ifeq ($(SRCARCH),x86)
+  ifeq (${IS_64_BIT}, 1)
+    LIBUNWIND_LIBS = -lunwind-x86_64 -lunwind -llzma
+  else
+    LIBUNWIND_LIBS = -lunwind-x86 -lunwind -llzma
+  endif
+endif
 ifneq ($(LIBUNWIND),1)
   NO_LIBUNWIND := 1
 endif
-
-ifndef NO_LIBUNWIND
-  ifeq ($(SRCARCH),arm)
-    LIBUNWIND_LIBS = -lunwind -lunwind-arm
-  endif
-  ifeq ($(SRCARCH),arm64)
-    LIBUNWIND_LIBS = -lunwind -lunwind-aarch64
-  endif
-  ifeq ($(SRCARCH),loongarch)
-    LIBUNWIND_LIBS = -lunwind -lunwind-loongarch64
-  endif
-  ifeq ($(ARCH),mips)
-    LIBUNWIND_LIBS = -lunwind -lunwind-mips
-  endif
-  ifeq ($(SRCARCH),powerpc)
-    LIBUNWIND_LIBS := -lunwind -lunwind-ppc64
-  endif
-  ifeq ($(SRCARCH),riscv)
-    LIBUNWIND_LIBS := -lunwind -lunwind-riscv
-  endif
-  ifeq ($(SRCARCH),s390)
-    LIBUNWIND_LIBS := -lunwind -lunwind-s390x
-  endif
-  ifeq ($(SRCARCH),x86)
-    ifeq (${IS_64_BIT}, 1)
-      LIBUNWIND_LIBS = -lunwind-x86_64 -lunwind -llzma
-    else
-      LIBUNWIND_LIBS = -lunwind-x86 -lunwind -llzma
-    endif
-  endif
-  ifeq ($(LIBUNWIND_LIBS),)
-    NO_LIBUNWIND := 1
-  endif
+ifeq ($(LIBUNWIND_LIBS),)
+  NO_LIBUNWIND := 1
 endif
 
 #
@@ -124,24 +125,24 @@ endif
 #
 LIBUNWIND_ARCHS:=aarch64 arm loongarch64 mips ppc32 ppc64 riscv s390x x86 x86_64
 
-ifndef NO_LIBUNWIND
-  FEATURE_CHECK_CFLAGS-libunwind = $(LIBUNWIND_CFLAGS)
-  FEATURE_CHECK_LDFLAGS-libunwind = $(LIBUNWIND_LDFLAGS) $(LIBUNWIND_LIBS)
-  FEATURE_CHECK_CFLAGS-libunwind-debug-frame = $(LIBUNWIND_CFLAGS)
-  FEATURE_CHECK_LDFLAGS-libunwind-debug-frame = $(LIBUNWIND_LDFLAGS) $(LIBUNWIND_LIBS)
-
-  ifdef LIBUNWIND_DIR
-    LIBUNWIND_CFLAGS  = -I$(LIBUNWIND_DIR)/include
-    LIBUNWIND_LDFLAGS = -L$(LIBUNWIND_DIR)/lib
-
-    define libunwind_arch_set_flags
-      FEATURE_CHECK_CFLAGS-libunwind-$(1)  = -I$(LIBUNWIND_DIR)/include
-      FEATURE_CHECK_LDFLAGS-libunwind-$(1) = -L$(LIBUNWIND_DIR)/lib -lunwind -lunwind-$(1)
-    endef
-    $(foreach arch,$(LIBUNWIND_ARCHS), \
-      $(eval $(call libunwind_arch_set_flags,$(arch))) \
-    )
-  endif
+# "Local" (no arch specified) feature test flags.
+FEATURE_CHECK_CFLAGS-libunwind = $(LIBUNWIND_CFLAGS)
+FEATURE_CHECK_LDFLAGS-libunwind = $(LIBUNWIND_LDFLAGS) $(LIBUNWIND_LIBS)
+FEATURE_CHECK_CFLAGS-libunwind-debug-frame = $(LIBUNWIND_CFLAGS)
+FEATURE_CHECK_LDFLAGS-libunwind-debug-frame = $(LIBUNWIND_LDFLAGS) $(LIBUNWIND_LIBS)
+
+# Add directory into the "remote" (build for a a specific arch) feature tests.
+ifdef LIBUNWIND_DIR
+  LIBUNWIND_CFLAGS  = -I$(LIBUNWIND_DIR)/include
+  LIBUNWIND_LDFLAGS = -L$(LIBUNWIND_DIR)/lib
+
+  define libunwind_arch_set_flags
+    FEATURE_CHECK_CFLAGS-libunwind-$(1)  = -I$(LIBUNWIND_DIR)/include
+    FEATURE_CHECK_LDFLAGS-libunwind-$(1) = -L$(LIBUNWIND_DIR)/lib -lunwind -lunwind-$(1)
+  endef
+  $(foreach arch,$(LIBUNWIND_ARCHS), \
+    $(eval $(call libunwind_arch_set_flags,$(arch))) \
+  )
 endif
 
 ifdef CSINCLUDES
-- 
2.54.0.746.g67dd491aae-goog


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

* [PATCH v1 2/3] perf kwork: Fix address sanitizer issues
  2026-05-21  7:24 [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
  2026-05-21  7:24 ` [PATCH v1 1/3] perf build: Unconditionally set up libunwind feature build flags Ian Rogers
@ 2026-05-21  7:24 ` Ian Rogers
  2026-05-21  7:24 ` [PATCH v1 3/3] perf kwork: Fix memory management of kwork_work Ian Rogers
  2026-05-22 21:33 ` [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
  3 siblings, 0 replies; 7+ messages in thread
From: Ian Rogers @ 2026-05-21  7:24 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	linux-kernel, linux-perf-users

There is a double free in the record array due to how parse_options
will mutate the array. Fix by keeping an array that isn't
mutated. Ensure kwork_usage is freed on all paths.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-kwork.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index 59a54d11f7fa..a4604e152002 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -2258,7 +2258,7 @@ static void setup_event_list(struct perf_kwork *kwork,
 static int perf_kwork__record(struct perf_kwork *kwork,
 			      int argc, const char **argv)
 {
-	const char **rec_argv;
+	const char **rec_argv, **to_free = NULL;
 	unsigned int rec_argc, i, j;
 	struct kwork_class *class;
 	int ret;
@@ -2295,16 +2295,27 @@ static int perf_kwork__record(struct perf_kwork *kwork,
 
 	BUG_ON(i != rec_argc);
 
+	/* Save the pointers as the array will be mutated by cmd_record. */
+	to_free = calloc(rec_argc + 1, sizeof(char *));
+	if (to_free == NULL) {
+		ret = -ENOMEM;
+		goto EXIT;
+	}
+
 	pr_debug("record comm: ");
-	for (j = 0; j < rec_argc; j++)
+	for (j = 0; j < rec_argc; j++) {
 		pr_debug("%s ", rec_argv[j]);
+		to_free[j] = rec_argv[j];
+	}
 	pr_debug("\n");
 
 	ret = cmd_record(i, rec_argv);
 
 EXIT:
 	for (i = 0; i < rec_argc; i++)
-		free((void *)rec_argv[i]);
+		free((void *)(to_free ? to_free[i] : rec_argv[i]));
+
+	free(to_free);
 	free(rec_argv);
 	return ret;
 }
@@ -2447,6 +2458,7 @@ int cmd_kwork(int argc, const char **argv)
 	const char *const kwork_subcommands[] = {
 		"record", "report", "latency", "timehist", "top", NULL
 	};
+	int ret = 0;
 
 	perf_tool__init(&kwork.tool, /*ordered_events=*/true);
 	kwork.tool.mmap	  = perf_event__process_mmap;
@@ -2463,7 +2475,7 @@ int cmd_kwork(int argc, const char **argv)
 
 	if (strlen(argv[0]) > 2 && strstarts("record", argv[0])) {
 		setup_event_list(&kwork, kwork_options, kwork_usage);
-		return perf_kwork__record(&kwork, argc, argv);
+		ret = perf_kwork__record(&kwork, argc, argv);
 	} else if (strlen(argv[0]) > 2 && strstarts("report", argv[0])) {
 		kwork.sort_order = default_report_sort_order;
 		if (argc > 1) {
@@ -2474,7 +2486,7 @@ int cmd_kwork(int argc, const char **argv)
 		kwork.report = KWORK_REPORT_RUNTIME;
 		setup_sorting(&kwork, report_options, report_usage);
 		setup_event_list(&kwork, kwork_options, kwork_usage);
-		return perf_kwork__report(&kwork);
+		ret = perf_kwork__report(&kwork);
 	} else if (strlen(argv[0]) > 2 && strstarts("latency", argv[0])) {
 		kwork.sort_order = default_latency_sort_order;
 		if (argc > 1) {
@@ -2485,7 +2497,7 @@ int cmd_kwork(int argc, const char **argv)
 		kwork.report = KWORK_REPORT_LATENCY;
 		setup_sorting(&kwork, latency_options, latency_usage);
 		setup_event_list(&kwork, kwork_options, kwork_usage);
-		return perf_kwork__report(&kwork);
+		ret = perf_kwork__report(&kwork);
 	} else if (strlen(argv[0]) > 2 && strstarts("timehist", argv[0])) {
 		if (argc > 1) {
 			argc = parse_options(argc, argv, timehist_options, timehist_usage, 0);
@@ -2494,7 +2506,7 @@ int cmd_kwork(int argc, const char **argv)
 		}
 		kwork.report = KWORK_REPORT_TIMEHIST;
 		setup_event_list(&kwork, kwork_options, kwork_usage);
-		return perf_kwork__timehist(&kwork);
+		ret = perf_kwork__timehist(&kwork);
 	} else if (strlen(argv[0]) > 2 && strstarts("top", argv[0])) {
 		kwork.sort_order = default_top_sort_order;
 		if (argc > 1) {
@@ -2507,12 +2519,12 @@ int cmd_kwork(int argc, const char **argv)
 			kwork.event_list_str = "sched, irq, softirq";
 		setup_event_list(&kwork, kwork_options, kwork_usage);
 		setup_sorting(&kwork, top_options, top_usage);
-		return perf_kwork__top(&kwork);
+		ret = perf_kwork__top(&kwork);
 	} else
 		usage_with_options(kwork_usage, kwork_options);
 
 	/* free usage string allocated by parse_options_subcommand */
 	free((void *)kwork_usage[0]);
 
-	return 0;
+	return ret;
 }
-- 
2.54.0.746.g67dd491aae-goog


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

* [PATCH v1 3/3] perf kwork: Fix memory management of kwork_work
  2026-05-21  7:24 [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
  2026-05-21  7:24 ` [PATCH v1 1/3] perf build: Unconditionally set up libunwind feature build flags Ian Rogers
  2026-05-21  7:24 ` [PATCH v1 2/3] perf kwork: Fix address sanitizer issues Ian Rogers
@ 2026-05-21  7:24 ` Ian Rogers
  2026-05-21  9:34   ` sashiko-bot
  2026-05-22 21:33 ` [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
  3 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2026-05-21  7:24 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	linux-kernel, linux-perf-users

This commit addresses several memory management issues in builtin-kwork.c:
1. Implements a global cleanup function perf_kwork__exit to free all
   kwork_work and kwork_atom_page objects at the end of the command.
2. Ensures all 'name' fields in struct kwork_work are malloc-ed (or NULL)
   and properly freed by using strdup and zfree.
3. Fixes memory leaks in top_merge_tasks where kwork_work objects were
   dropped without being freed.
4. Adds robustness with NULL checks for name fields.
5. Fixes workqueue_work_init to correctly resolve and strdup kernel
   function names, preventing bad-free errors.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-kwork.c  | 121 ++++++++++++++++++++++++++++--------
 tools/perf/util/bpf_kwork.c |  14 +++--
 tools/perf/util/kwork.h     |   9 +++
 3 files changed, 115 insertions(+), 29 deletions(-)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index a4604e152002..f793ea578515 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -323,8 +323,8 @@ static struct kwork_work *work_search(struct rb_root_cached *root,
 		else if (cmp < 0)
 			node = node->rb_right;
 		else {
-			if (work->name == NULL)
-				work->name = key->name;
+			if (work->name == NULL && key->name != NULL)
+				work->name = strdup(key->name);
 			return work;
 		}
 	}
@@ -371,11 +371,54 @@ static struct kwork_work *work_new(struct kwork_work *key)
 
 	work->id = key->id;
 	work->cpu = key->cpu;
-	work->name = key->name;
+	work->name = key->name ? strdup(key->name) : NULL;
 	work->class = key->class;
 	return work;
 }
 
+
+static void work_delete(struct kwork_work *work)
+{
+	if (work) {
+		work_exit(work);
+		free(work);
+	}
+}
+
+static void kwork_work__free_root(struct rb_root_cached *root)
+{
+	struct rb_node *next;
+	struct kwork_work *work;
+
+	while ((next = rb_first_cached(root))) {
+		work = rb_entry(next, struct kwork_work, node);
+		rb_erase_cached(next, root);
+		work_delete(work);
+	}
+}
+
+static void perf_kwork__exit(struct perf_kwork *kwork)
+{
+	struct kwork_class *class;
+	struct kwork_atom_page *page, *tmp_page;
+
+	list_for_each_entry(class, &kwork->class_list, list) {
+		kwork_work__free_root(&class->work_root);
+	}
+
+	kwork_work__free_root(&kwork->sorted_work_root);
+
+	list_for_each_entry_safe(page, tmp_page, &kwork->atom_page_list, list) {
+		list_del_init(&page->list);
+		free(page);
+	}
+
+	INIT_LIST_HEAD(&kwork->class_list);
+	INIT_LIST_HEAD(&kwork->atom_page_list);
+	INIT_LIST_HEAD(&kwork->sort_list);
+	INIT_LIST_HEAD(&kwork->cmp_id);
+}
+
 static struct kwork_work *work_findnew(struct rb_root_cached *root,
 				       struct kwork_work *key,
 				       struct list_head *sort_list)
@@ -453,25 +496,29 @@ static int work_push_atom(struct perf_kwork *kwork,
 			  struct kwork_work **ret_work,
 			  bool overwrite)
 {
-	struct kwork_atom *atom, *dst_atom, *last_atom;
+	struct kwork_atom *atom = NULL, *dst_atom, *last_atom;
 	struct kwork_work *work, key;
+	int ret = 0;
 
 	BUG_ON(class->work_init == NULL);
 	class->work_init(kwork, class, &key, src_type, sample, machine);
 
 	atom = atom_new(kwork, sample);
-	if (atom == NULL)
-		return -1;
+	if (atom == NULL) {
+		ret = -1;
+		goto out;
+	}
 
 	work = work_findnew(&class->work_root, &key, &kwork->cmp_id);
 	if (work == NULL) {
 		atom_free(atom);
-		return -1;
+		ret = -1;
+		goto out;
 	}
 
 	if (!profile_event_match(kwork, work, sample)) {
 		atom_free(atom);
-		return 0;
+		goto out;
 	}
 
 	if (dst_type < KWORK_TRACE_MAX) {
@@ -498,8 +545,9 @@ static int work_push_atom(struct perf_kwork *kwork,
 	}
 
 	list_add_tail(&atom->list, &work->atom_list[src_type]);
-
-	return 0;
+out:
+	work_exit(&key);
+	return ret;
 }
 
 static struct kwork_atom *work_pop_atom(struct perf_kwork *kwork,
@@ -510,7 +558,7 @@ static struct kwork_atom *work_pop_atom(struct perf_kwork *kwork,
 					struct machine *machine,
 					struct kwork_work **ret_work)
 {
-	struct kwork_atom *atom, *src_atom;
+	struct kwork_atom *atom = NULL, *src_atom;
 	struct kwork_work *work, key;
 
 	BUG_ON(class->work_init == NULL);
@@ -521,15 +569,15 @@ static struct kwork_atom *work_pop_atom(struct perf_kwork *kwork,
 		*ret_work = work;
 
 	if (work == NULL)
-		return NULL;
+		goto out;
 
 	if (!profile_event_match(kwork, work, sample))
-		return NULL;
+		goto out;
 
 	atom = list_last_entry_or_null(&work->atom_list[dst_type],
 				       struct kwork_atom, list);
 	if (atom != NULL)
-		return atom;
+		goto out;
 
 	src_atom = atom_new(kwork, sample);
 	if (src_atom != NULL)
@@ -538,8 +586,9 @@ static struct kwork_atom *work_pop_atom(struct perf_kwork *kwork,
 		if (ret_work != NULL)
 			*ret_work = NULL;
 	}
-
-	return NULL;
+out:
+	work_exit(&key);
+	return atom;
 }
 
 static struct kwork_work *find_work_by_id(struct rb_root_cached *root,
@@ -1002,13 +1051,16 @@ static void irq_work_init(struct perf_kwork *kwork,
 		work->name = NULL;
 	} else {
 		work->id = perf_sample__intval(sample, "irq");
-		work->name = perf_sample__strval(sample, "name");
+		work->name = strdup(perf_sample__strval(sample, "name") ?: "<unknown>");
 	}
 }
 
 static void irq_work_name(struct kwork_work *work, char *buf, int len)
 {
-	snprintf(buf, len, "%s:%" PRIu64 "", work->name, work->id);
+	if (work->name != NULL)
+		snprintf(buf, len, "%s:%" PRIu64 "", work->name, work->id);
+	else
+		snprintf(buf, len, "%" PRIu64 "", work->id);
 }
 
 static struct kwork_class kwork_irq = {
@@ -1135,7 +1187,10 @@ static void softirq_work_init(struct perf_kwork *kwork,
 
 static void softirq_work_name(struct kwork_work *work, char *buf, int len)
 {
-	snprintf(buf, len, "(s)%s:%" PRIu64 "", work->name, work->id);
+	if (work->name != NULL)
+		snprintf(buf, len, "(s)%s:%" PRIu64 "", work->name, work->id);
+	else
+		snprintf(buf, len, "(s)%" PRIu64 "", work->id);
 }
 
 static struct kwork_class kwork_softirq = {
@@ -1220,8 +1275,14 @@ static void workqueue_work_init(struct perf_kwork *kwork __maybe_unused,
 	work->class = class;
 	work->cpu = sample->cpu;
 	work->id = perf_sample__intval(sample, "work");
-	work->name = function_addr == 0 ? NULL :
-		machine__resolve_kernel_addr(machine, &function_addr, &modp);
+	work->name = NULL;
+
+	if (function_addr != 0) {
+		const char *name = machine__resolve_kernel_addr(machine, &function_addr, &modp);
+
+		if (name)
+			work->name = strdup(name);
+	}
 }
 
 static void workqueue_work_name(struct kwork_work *work, char *buf, int len)
@@ -1284,16 +1345,16 @@ static void sched_work_init(struct perf_kwork *kwork __maybe_unused,
 
 	if (src_type == KWORK_TRACE_EXIT) {
 		work->id = perf_sample__intval(sample, "prev_pid");
-		work->name = strdup(perf_sample__strval(sample, "prev_comm"));
+		work->name = strdup(perf_sample__strval(sample, "prev_comm") ?: "<unknown>");
 	} else if (src_type == KWORK_TRACE_ENTRY) {
 		work->id = perf_sample__intval(sample, "next_pid");
-		work->name = strdup(perf_sample__strval(sample, "next_comm"));
+		work->name = strdup(perf_sample__strval(sample, "next_comm") ?: "<unknown>");
 	}
 }
 
 static void sched_work_name(struct kwork_work *work, char *buf, int len)
 {
-	snprintf(buf, len, "%s", work->name);
+	snprintf(buf, len, "%s", work->name ?: "");
 }
 
 static struct kwork_class kwork_sched = {
@@ -2100,8 +2161,10 @@ static void top_merge_tasks(struct perf_kwork *kwork)
 		rb_erase_cached(node, &class->work_root);
 		data = rb_entry(node, struct kwork_work, node);
 
-		if (!profile_name_match(kwork, data))
+		if (!profile_name_match(kwork, data)) {
+			work_delete(data);
 			continue;
+		}
 
 		cpu = data->cpu;
 		merged_work = find_work_by_id(&merged_root, data->id,
@@ -2109,11 +2172,17 @@ static void top_merge_tasks(struct perf_kwork *kwork)
 		if (!merged_work) {
 			work_insert(&merged_root, data, &kwork->cmp_id);
 		} else {
+			if (merged_work->name == NULL && data->name != NULL)
+				merged_work->name = strdup(data->name);
+
 			merged_work->total_runtime += data->total_runtime;
 			merged_work->cpu_usage += data->cpu_usage;
 		}
 
 		top_calc_load_runtime(kwork, data);
+
+		if (merged_work)
+			work_delete(data);
 	}
 
 	work_sort(kwork, class, &merged_root);
@@ -2523,6 +2592,8 @@ int cmd_kwork(int argc, const char **argv)
 	} else
 		usage_with_options(kwork_usage, kwork_options);
 
+	perf_kwork__exit(&kwork);
+
 	/* free usage string allocated by parse_options_subcommand */
 	free((void *)kwork_usage[0]);
 
diff --git a/tools/perf/util/bpf_kwork.c b/tools/perf/util/bpf_kwork.c
index d3a2e548f2b6..2248f462a847 100644
--- a/tools/perf/util/bpf_kwork.c
+++ b/tools/perf/util/bpf_kwork.c
@@ -273,6 +273,7 @@ static int add_work(struct perf_kwork *kwork,
 		.cpu = key->cpu,
 	};
 	enum kwork_class_type type = key->type;
+	int ret = 0;
 
 	if (!valid_kwork_class_type(type)) {
 		pr_debug("Invalid class type %d to add work\n", type);
@@ -287,8 +288,10 @@ static int add_work(struct perf_kwork *kwork,
 		return -1;
 
 	work = kwork->add_work(kwork, tmp.class, &tmp);
-	if (work == NULL)
-		return -1;
+	if (work == NULL) {
+		ret = -1;
+		goto out;
+	}
 
 	if (kwork->report == KWORK_REPORT_RUNTIME) {
 		work->nr_atoms = data->nr;
@@ -304,13 +307,16 @@ static int add_work(struct perf_kwork *kwork,
 		work->max_latency_end = data->max_time_end;
 	} else {
 		pr_debug("Invalid bpf report type %d\n", kwork->report);
-		return -1;
+		ret = -1;
+		goto out;
 	}
 
 	kwork->timestart = (u64)ts_start.tv_sec * NSEC_PER_SEC + ts_start.tv_nsec;
 	kwork->timeend = (u64)ts_end.tv_sec * NSEC_PER_SEC + ts_end.tv_nsec;
 
-	return 0;
+out:
+	work_exit(&tmp);
+	return ret;
 }
 
 int perf_kwork__report_read_bpf(struct perf_kwork *kwork)
diff --git a/tools/perf/util/kwork.h b/tools/perf/util/kwork.h
index abf637d44794..81d39a7f78c8 100644
--- a/tools/perf/util/kwork.h
+++ b/tools/perf/util/kwork.h
@@ -5,6 +5,7 @@
 #include "util/tool.h"
 #include "util/time-utils.h"
 
+#include <stdlib.h>
 #include <linux/bitmap.h>
 #include <linux/list.h>
 #include <linux/rbtree.h>
@@ -164,6 +165,14 @@ struct kwork_class {
 			  char *buf, int len);
 };
 
+static inline void work_exit(struct kwork_work *work)
+{
+	if (work) {
+		free(work->name);
+		work->name = NULL;
+	}
+}
+
 struct trace_kwork_handler {
 	int (*raise_event)(struct perf_kwork *kwork,
 			   struct kwork_class *class,
-- 
2.54.0.746.g67dd491aae-goog


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

* Re: [PATCH v1 3/3] perf kwork: Fix memory management of kwork_work
  2026-05-21  7:24 ` [PATCH v1 3/3] perf kwork: Fix memory management of kwork_work Ian Rogers
@ 2026-05-21  9:34   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-05-21  9:34 UTC (permalink / raw)
  To: Ian Rogers; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The top_print_work function passes work->name to printf without verifying it is non-NULL, which can cause a crash.
--

commit 03666bc88a2d8e65da244923eb1ef63e0a473153
Author: Ian Rogers <irogers@google.com>

perf kwork: Fix memory management of kwork_work

This commit implements a global cleanup function to free all kwork_work
and kwork_atom_page objects at exit. It also fixes memory leaks during
task merging and ensures all name fields in struct kwork_work are properly
allocated and freed, while adding robustness checks for NULL names.

> diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
> index a4604e1520023..f793ea578515d 100644
> --- a/tools/perf/builtin-kwork.c
> +++ b/tools/perf/builtin-kwork.c
[ ... ]
> @@ -1002,13 +1051,16 @@ static void irq_work_init(struct perf_kwork *kwork,
>  		work->name = NULL;
>  	} else {
>  		work->id = perf_sample__intval(sample, "irq");
> -		work->name = perf_sample__strval(sample, "name");
> +		work->name = strdup(perf_sample__strval(sample, "name") ?: "<unknown>");
>  	}
>  }
>  
>  static void irq_work_name(struct kwork_work *work, char *buf, int len)
>  {
> -	snprintf(buf, len, "%s:%" PRIu64 "", work->name, work->id);
> +	if (work->name != NULL)
> +		snprintf(buf, len, "%s:%" PRIu64 "", work->name, work->id);
> +	else
> +		snprintf(buf, len, "%" PRIu64 "", work->id);
>  }

[Severity: Medium]
This is a pre-existing issue, but while you are adding NULL checks for
work->name here, should top_print_work() also be updated?

If strdup() fails due to memory pressure, or if bpf_map_lookup_elem() fails
in BPF mode, work->name can be NULL. In top_print_work(), work->name is
passed directly to printf() as a string, which can cause a segmentation fault
on strict libc implementations:

tools/perf/builtin-kwork.c:top_print_work() {
    ...
	if (kwork->use_bpf)
		ret += printf(" %s%s%s",
			      work->is_kthread ? "[" : "",
			      work->name,
			      work->is_kthread ? "]" : "");
	else
		ret += printf(" %-*s", PRINT_TASK_NAME_WIDTH, work->name);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521072429.1283423-1-irogers@google.com?part=3

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

* Re: [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds
  2026-05-21  7:24 [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
                   ` (2 preceding siblings ...)
  2026-05-21  7:24 ` [PATCH v1 3/3] perf kwork: Fix memory management of kwork_work Ian Rogers
@ 2026-05-22 21:33 ` Ian Rogers
  2026-05-22 23:52   ` Arnaldo Carvalho de Melo
  3 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2026-05-22 21:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Thomas Richter
  Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, James Clark,
	Adrian Hunter, linux-perf-users, linux-kernel

On Thu, May 21, 2026 at 12:24 AM Ian Rogers <irogers@google.com> wrote:
>
> The combination of the libunwind refactoring and build system changes
> exposed a build-test regression for the libunwind build using a cached
> feature dump. Re-send kwork fixes [1] moving a definition to kwork.h
> to avoid breaking no libbpf builds.
>
> 1. Build Fix for libunwind:
>    "make feature-dump" runs without LIBUNWIND=1, skipping the setup of
>    libunwind LDFLAGS. When Makefile.feature is included to generate the
>    cached FEATURE-DUMP, the local test-libunwind.bin compiles without
>    arch-specific link flags (-lunwind-x86_64) and fails, caching a false
>    negative ("feature-libunwind=0"). Later builds (such as make_libunwind_O
>    reusing the dump) compile the unwinder based on CONFIG_LIBUNWIND (remote)
>    but disable HAVE_LIBUNWIND_SUPPORT (local), breaking compilation due to
>    missing maps__e_machine declarations in maps.h. Fixed by unconditionally
>    setting up libunwind feature build flags in Makefile.config.
>
> 2. kwork ASAN Double-Free & Leaks:
>    - Solves double-free in the record command argument array due to
>      parse_options mutation, by preserving original pointers in a calloc'ed
>      to_free buffer.
>    - Ensures kwork_usage string is freed on all exit paths.
>
> 3. kwork Unified Memory Lifecycle:
>    - Establishes a unified memory ownership model for kwork_work by
>      ensuring work names are dynamically duplicated via strdup() and
>      consistently freed using work_exit() and a central perf_kwork__exit()
>      teardown path.

It would be nice to land these fixes. Thomas reported the kwork crashes in:
https://lore.kernel.org/linux-perf-users/314df838-4c38-4f03-9515-ae1dabd09a54@linux.ibm.com/

Thanks,
Ian

> [1] https://lore.kernel.org/linux-perf-users/20260520190538.142018-32-irogers@google.com/
>
> Ian Rogers (3):
>   perf build: Unconditionally set up libunwind feature build flags
>   perf kwork: Fix address sanitizer issues
>   perf kwork: Fix memory management of kwork_work
>
>  tools/perf/Makefile.config  | 103 ++++++++++++------------
>  tools/perf/builtin-kwork.c  | 151 ++++++++++++++++++++++++++++--------
>  tools/perf/util/bpf_kwork.c |  14 +++-
>  tools/perf/util/kwork.h     |   9 +++
>  4 files changed, 188 insertions(+), 89 deletions(-)
>
> --
> 2.54.0.746.g67dd491aae-goog
>

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

* Re: [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds
  2026-05-22 21:33 ` [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
@ 2026-05-22 23:52   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-05-22 23:52 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Thomas Richter, Peter Zijlstra, Ingo Molnar,
	Jiri Olsa, James Clark, Adrian Hunter, linux-perf-users,
	linux-kernel

On Fri, May 22, 2026 at 02:33:05PM -0700, Ian Rogers wrote:
> On Thu, May 21, 2026 at 12:24 AM Ian Rogers <irogers@google.com> wrote:
> >
> > The combination of the libunwind refactoring and build system changes
> > exposed a build-test regression for the libunwind build using a cached
> > feature dump. Re-send kwork fixes [1] moving a definition to kwork.h
> > to avoid breaking no libbpf builds.
> >
> > 1. Build Fix for libunwind:
> >    "make feature-dump" runs without LIBUNWIND=1, skipping the setup of
> >    libunwind LDFLAGS. When Makefile.feature is included to generate the
> >    cached FEATURE-DUMP, the local test-libunwind.bin compiles without
> >    arch-specific link flags (-lunwind-x86_64) and fails, caching a false
> >    negative ("feature-libunwind=0"). Later builds (such as make_libunwind_O
> >    reusing the dump) compile the unwinder based on CONFIG_LIBUNWIND (remote)
> >    but disable HAVE_LIBUNWIND_SUPPORT (local), breaking compilation due to
> >    missing maps__e_machine declarations in maps.h. Fixed by unconditionally
> >    setting up libunwind feature build flags in Makefile.config.
> >
> > 2. kwork ASAN Double-Free & Leaks:
> >    - Solves double-free in the record command argument array due to
> >      parse_options mutation, by preserving original pointers in a calloc'ed
> >      to_free buffer.
> >    - Ensures kwork_usage string is freed on all exit paths.
> >
> > 3. kwork Unified Memory Lifecycle:
> >    - Establishes a unified memory ownership model for kwork_work by
> >      ensuring work names are dynamically duplicated via strdup() and
> >      consistently freed using work_exit() and a central perf_kwork__exit()
> >      teardown path.
> 
> It would be nice to land these fixes. Thomas reported the kwork crashes in:
> https://lore.kernel.org/linux-perf-users/314df838-4c38-4f03-9515-ae1dabd09a54@linux.ibm.com/

Thanks, applied to perf-tools-next, for v7.2.

- Arnaldo

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

end of thread, other threads:[~2026-05-22 23:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21  7:24 [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
2026-05-21  7:24 ` [PATCH v1 1/3] perf build: Unconditionally set up libunwind feature build flags Ian Rogers
2026-05-21  7:24 ` [PATCH v1 2/3] perf kwork: Fix address sanitizer issues Ian Rogers
2026-05-21  7:24 ` [PATCH v1 3/3] perf kwork: Fix memory management of kwork_work Ian Rogers
2026-05-21  9:34   ` sashiko-bot
2026-05-22 21:33 ` [PATCH v1 0/3] perf: Fix kwork memory sanitization and libunwind test builds Ian Rogers
2026-05-22 23:52   ` Arnaldo Carvalho de Melo

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