[PATCH v5 0/3] tools/mm/page_owner_sort: fix filtering and cleanup issues

[PATCH v5 0/3] tools/mm/page_owner_sort: fix filtering and cleanup issues

[Yichong Chen]

Hi,

Resend the page_owner_sort cleanup series with corrected threading.

This version replaces the single patch currently queued in mm-new by
splitting the cleanup into smaller patches and adding the search_pattern()
bounds fix.

Patch 1 renames is_need() to filter_record() and makes the filter path
return explicit results. Patch 2 fixes the per-record allocation leaks.
Patch 3 bounds search_pattern() output copies, addressing the pre-existing
issue reported by Sashiko/AI review.

Changes in v5:
- Resend the series with corrected threading.
- Keep Ye Liu and Zhen Ni copied, as Andrew requested.
- Add patch 3 to bound search_pattern() output copies.

Changes in v4:
- Split the filter_record() cleanup into a separate patch.
- Mention the is_need() to filter_record() rename in the commit log.
- Add Reviewed-by from Vishal.

Changes in v3:
- Keep the free in add_list(), since incomplete records are not included
  in list_size and cannot be reached by the common cleanup path.
- Return explicit error, skip, and match results from filter_record() to
  handle a NULL return from get_comm().

Changes in v2:
- Wrap commit message lines to approximately 75 columns.
- Use "Yichong Chen" as the author name.

Yichong Chen (3):
  tools/mm/page_owner_sort: return explicit filter results
  tools/mm/page_owner_sort: free per-record allocations
  tools/mm/page_owner_sort: bound pattern output copies

 tools/mm/page_owner_sort.c | 86 ++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 17 deletions(-)

-- 
2.51.0

[PATCH v5 1/3] tools/mm/page_owner_sort: return explicit filter results

[Yichong Chen]

Rename is_need() to filter_record() and make the filter path return
explicit error, skip, and match results. This lets callers distinguish
allocation failures from records that simply do not match active filters.

Reviewed-by: Vishal Moola <vishal.moola@gmail.com>
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 tools/mm/page_owner_sort.c | 40 +++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index e6954909401c..3c754826cec5 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -43,6 +43,13 @@ enum FILTER_BIT {
 	FILTER_TGID = 1<<2,
 	FILTER_COMM = 1<<3
 };
+
+enum FILTER_RESULT {
+	FILTER_ERROR,
+	FILTER_SKIP,
+	FILTER_MATCH
+};
+
 enum CULL_BIT {
 	CULL_PID = 1<<1,
 	CULL_TGID = 1<<2,
@@ -372,6 +379,9 @@ static char *get_comm(char *buf)
 {
 	char *comm_str = malloc(TASK_COMM_LEN);
 
+	if (!comm_str)
+		return NULL;
+
 	memset(comm_str, 0, TASK_COMM_LEN);
 
 	search_pattern(&comm_pattern, comm_str, buf);
@@ -450,32 +460,44 @@ static bool match_str_list(const char *str, char **list, int list_size)
 	return false;
 }
 
-static bool is_need(char *buf)
+static enum FILTER_RESULT filter_record(char *buf)
 {
+	char *comm;
+
 	if ((filter & FILTER_PID) && !match_num_list(get_pid(buf), fc.pids, fc.pids_size))
-		return false;
+		return FILTER_SKIP;
 	if ((filter & FILTER_TGID) &&
 		!match_num_list(get_tgid(buf), fc.tgids, fc.tgids_size))
-		return false;
+		return FILTER_SKIP;
+	if (!(filter & FILTER_COMM))
+		return FILTER_MATCH;
 
-	char *comm = get_comm(buf);
+	comm = get_comm(buf);
+	if (!comm)
+		return FILTER_ERROR;
 
-	if ((filter & FILTER_COMM) &&
-	!match_str_list(comm, fc.comms, fc.comms_size)) {
+	if (!match_str_list(comm, fc.comms, fc.comms_size)) {
 		free(comm);
-		return false;
+		return FILTER_SKIP;
 	}
 	free(comm);
-	return true;
+	return FILTER_MATCH;
 }
 
 static bool add_list(char *buf, int len, char *ext_buf)
 {
+	enum FILTER_RESULT filter_result;
+
 	if (list_size == max_size) {
 		fprintf(stderr, "max_size too small??\n");
 		return false;
 	}
-	if (!is_need(buf))
+	filter_result = filter_record(buf);
+	if (filter_result == FILTER_ERROR) {
+		fprintf(stderr, "Out of memory\n");
+		return false;
+	}
+	if (filter_result == FILTER_SKIP)
 		return true;
 	list[list_size].pid = get_pid(buf);
 	list[list_size].tgid = get_tgid(buf);
-- 
2.51.0

[PATCH v5 2/3] tools/mm/page_owner_sort: free per-record allocations

[Yichong Chen]

add_list() allocates comm and txt for each page owner record, but the
cleanup path only frees the outer list array. This leaks both buffers for
every retained record.

Free partial allocations in add_list(), discarded records during
culling, and retained records on exit.

Reviewed-by: Vishal Moola <vishal.moola@gmail.com>
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 tools/mm/page_owner_sort.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index 3c754826cec5..4c9be28abe3b 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -396,6 +396,12 @@ static char *get_comm(char *buf)
 	return comm_str;
 }
 
+static void free_block_list(struct block_list *block)
+{
+	free(block->comm);
+	free(block->txt);
+}
+
 static int get_arg_type(const char *arg)
 {
 	if (!strcmp(arg, "pid") || !strcmp(arg, "p"))
@@ -502,9 +508,14 @@ static bool add_list(char *buf, int len, char *ext_buf)
 	list[list_size].pid = get_pid(buf);
 	list[list_size].tgid = get_tgid(buf);
 	list[list_size].comm = get_comm(buf);
-	list[list_size].txt = malloc(len+1);
+	if (!list[list_size].comm) {
+		fprintf(stderr, "Out of memory\n");
+		return false;
+	}
+	list[list_size].txt = malloc(len + 1);
 	if (!list[list_size].txt) {
 		fprintf(stderr, "Out of memory\n");
+		free(list[list_size].comm);
 		return false;
 	}
 	memcpy(list[list_size].txt, buf, len);
@@ -863,8 +874,10 @@ int main(int argc, char **argv)
 		} else {
 			list[count-1].num += list[i].num;
 			list[count-1].page_num += list[i].page_num;
+			free_block_list(&list[i]);
 		}
 	}
+	list_size = count;
 
 	qsort(list, count, sizeof(list[0]), compare_sort_condition);
 
@@ -898,8 +911,11 @@ int main(int argc, char **argv)
 		free(ext_buf);
 	if (buf)
 		free(buf);
-	if (list)
+	if (list) {
+		for (i = 0; i < list_size; i++)
+			free_block_list(&list[i]);
 		free(list);
+	}
 out_ts:
 	regfree(&ts_nsec_pattern);
 out_comm:
-- 
2.51.0

[PATCH v5 3/3] tools/mm/page_owner_sort: bound pattern output copies

[Yichong Chen]

search_pattern() copies a regex capture into caller-provided buffers
without knowing their sizes. Several callers pass fixed-size buffers,
including FIELD_BUFF and TASK_COMM_LEN.

Pass the destination size to search_pattern(), reject captures that do
not fit before copying them, and terminate the output string inside
search_pattern().

Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 tools/mm/page_owner_sort.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index 4c9be28abe3b..35d3d254941c 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -237,7 +237,8 @@ static int remove_pattern(regex_t *pattern, char *buf, int len)
 	return len - (pmatch[1].rm_eo - pmatch[1].rm_so);
 }
 
-static int search_pattern(regex_t *pattern, char *pattern_str, char *buf)
+static int search_pattern(regex_t *pattern, char *pattern_str,
+			  size_t pattern_str_size, char *buf)
 {
 	int err, val_len;
 	regmatch_t pmatch[2];
@@ -249,6 +250,12 @@ static int search_pattern(regex_t *pattern, char *pattern_str, char *buf)
 		return -1;
 	}
 	val_len = pmatch[1].rm_eo - pmatch[1].rm_so;
+	if ((size_t)val_len >= pattern_str_size) {
+		if (debug_on)
+			fprintf(stderr, "pattern too long in %s\n", buf);
+		return -1;
+	}
 
 	memcpy(pattern_str, buf + pmatch[1].rm_so, val_len);
+	pattern_str[val_len] = '\0';
 
@@ -307,7 +314,8 @@ static int get_page_num(char *buf)
 	char order_str[FIELD_BUFF] = {0};
 	char *endptr;
 
-	search_pattern(&order_pattern, order_str, buf);
+	if (search_pattern(&order_pattern, order_str, sizeof(order_str), buf) < 0)
+		return 0;
 	errno = 0;
 	order_val = strtol(order_str, &endptr, 10);
 	if (order_val > 64 || errno != 0 || endptr == order_str || *endptr != '\0') {
@@ -325,7 +333,8 @@ static pid_t get_pid(char *buf)
 	char pid_str[FIELD_BUFF] = {0};
 	char *endptr;
 
-	search_pattern(&pid_pattern, pid_str, buf);
+	if (search_pattern(&pid_pattern, pid_str, sizeof(pid_str), buf) < 0)
+		return -1;
 	errno = 0;
 	pid = strtol(pid_str, &endptr, 10);
 	if (errno != 0 || endptr == pid_str || *endptr != '\0') {
@@ -344,7 +353,8 @@ static pid_t get_tgid(char *buf)
 	char tgid_str[FIELD_BUFF] = {0};
 	char *endptr;
 
-	search_pattern(&tgid_pattern, tgid_str, buf);
+	if (search_pattern(&tgid_pattern, tgid_str, sizeof(tgid_str), buf) < 0)
+		return -1;
 	errno = 0;
 	tgid = strtol(tgid_str, &endptr, 10);
 	if (errno != 0 || endptr == tgid_str || *endptr != '\0') {
@@ -363,7 +373,9 @@ static __u64 get_ts_nsec(char *buf)
 	char ts_nsec_str[FIELD_BUFF] = {0};
 	char *endptr;
 
-	search_pattern(&ts_nsec_pattern, ts_nsec_str, buf);
+	if (search_pattern(&ts_nsec_pattern, ts_nsec_str,
+			   sizeof(ts_nsec_str), buf) < 0)
+		return -1;
 	errno = 0;
 	ts_nsec = strtoull(ts_nsec_str, &endptr, 10);
 	if (errno != 0 || endptr == ts_nsec_str || *endptr != '\0') {
@@ -384,7 +396,10 @@ static char *get_comm(char *buf)
 
 	memset(comm_str, 0, TASK_COMM_LEN);
 
-	search_pattern(&comm_pattern, comm_str, buf);
+	if (search_pattern(&comm_pattern, comm_str, TASK_COMM_LEN, buf) < 0) {
+		free(comm_str);
+		return NULL;
+	}
 	errno = 0;
 	if (errno != 0) {
 		if (debug_on)
-- 
2.51.0

Re: [PATCH v5 0/3] tools/mm/page_owner_sort: fix filtering and cleanup issues

[Andrew Morton]

On Mon, 29 Jun 2026 09:43:13 +0800 Yichong Chen <chenyichong@uniontech.com> wrote:

> Resend the page_owner_sort cleanup series with corrected threading.
> 
> This version replaces the single patch currently queued in mm-new by
> splitting the cleanup into smaller patches and adding the search_pattern()
> bounds fix.
> 
> Patch 1 renames is_need() to filter_record() and makes the filter path
> return explicit results. Patch 2 fixes the per-record allocation leaks.
> Patch 3 bounds search_pattern() output copies, addressing the pre-existing
> issue reported by Sashiko/AI review.

Thanks, I'll add this for testing and further review.

It looks like AI review found a minorish issue:

https://sashiko.dev/#/patchset/20260629014316.130307-1-chenyichong@uniontech.com

[PATCH] tools/mm/page_owner_sort: report get_comm failures at source

[Yichong Chen]

get_comm() clears errno and checks it immediately, without any
operation that can set errno in between. The check can never be
true.

Print the concrete get_comm() failure reason where it is detected,
and let callers handle a NULL return without guessing the error
cause.

Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 tools/mm/page_owner_sort.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index 35d3d254941c..30b3cfc7eeab 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -391,17 +391,14 @@ static char *get_comm(char *buf)
 {
 	char *comm_str = malloc(TASK_COMM_LEN);
 
-	if (!comm_str)
+	if (!comm_str) {
+		fprintf(stderr, "Out of memory\n");
 		return NULL;
+	}
 
 	memset(comm_str, 0, TASK_COMM_LEN);
 
 	if (search_pattern(&comm_pattern, comm_str, TASK_COMM_LEN, buf) < 0) {
-		free(comm_str);
-		return NULL;
-	}
-	errno = 0;
-	if (errno != 0) {
 		if (debug_on)
 			fprintf(stderr, "wrong comm in follow buf:\n%s\n", buf);
 		free(comm_str);
@@ -514,19 +511,15 @@ static bool add_list(char *buf, int len, char *ext_buf)
 		return false;
 	}
 	filter_result = filter_record(buf);
-	if (filter_result == FILTER_ERROR) {
-		fprintf(stderr, "Out of memory\n");
+	if (filter_result == FILTER_ERROR)
 		return false;
-	}
 	if (filter_result == FILTER_SKIP)
 		return true;
 	list[list_size].pid = get_pid(buf);
 	list[list_size].tgid = get_tgid(buf);
 	list[list_size].comm = get_comm(buf);
-	if (!list[list_size].comm) {
-		fprintf(stderr, "Out of memory\n");
+	if (!list[list_size].comm)
 		return false;
-	}
 	list[list_size].txt = malloc(len + 1);
 	if (!list[list_size].txt) {
 		fprintf(stderr, "Out of memory\n");
-- 
2.51.0

Re: [PATCH] tools/mm/page_owner_sort: report get_comm failures at source

[Andrew Morton]

On Mon, 29 Jun 2026 14:25:46 +0800 Yichong Chen <chenyichong@uniontech.com> wrote:

> get_comm() clears errno and checks it immediately, without any
> operation that can set errno in between. The check can never be
> true.
> 
> Print the concrete get_comm() failure reason where it is detected,
> and let callers handle a NULL return without guessing the error
> cause.

Thanks.

AI review has an observation:
	https://sashiko.dev/#/patchset/20260629062546.141659-1-chenyichong@uniontech.com

Re: [PATCH] tools/mm/page_owner_sort: report get_comm failures at source

[Yichong Chen]

Hi Andrew,

Thanks for pointing this out.

I think keeping the parse-failure message under debug_on is acceptable here.
The failure still propagates through the existing NULL return path, and the
malformed-record dump is diagnostic output which matches the current debug
behavior.

If you prefer reporting this in normal mode as well, I think the minimal
change would be to drop the if (debug_on) guard before the existing error
message. Otherwise, I would prefer to keep the patch as-is.

Thanks,
Yichong