[PATCH] tools/mm: prevent page_owner_sort from truncating input

[PATCH] tools/mm: prevent page_owner_sort from truncating input

[Warren Xiong]

page_owner_sort opens the output file with "w" before reading the input.
If both paths refer to the same file, this truncates the input and the
tool silently processes zero records before returning success.

Open the output without truncating it and compare the device and inode
numbers of the opened files. Reject matching regular files, then truncate
regular output files only after the check. This also detects hard link and
symbolic link aliases while preserving non-regular outputs such as
/dev/stdout.

Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
---
 tools/mm/page_owner_sort.c | 41 ++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index 35d3d2549..602f541d8 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -713,7 +713,8 @@ int main(int argc, char **argv)
 	FILE *fin, *fout;
 	char *buf, *ext_buf;
 	int i, count, compare_flag;
-	struct stat st;
+	int fout_fd;
+	struct stat st, output_st;
 	int opt;
 	struct option longopts[] = {
 		{ "pid", required_argument, NULL, 1 },
@@ -836,13 +837,46 @@ int main(int argc, char **argv)
 	}
 
 	fin = fopen(argv[optind], "r");
-	fout = fopen(argv[optind + 1], "w");
-	if (!fin || !fout) {
+	if (!fin) {
 		usage();
 		perror("open: ");
 		exit(1);
 	}
 
+	if (fstat(fileno(fin), &st)) {
+		perror("fstat input");
+		exit(1);
+	}
+
+	/*
+	 * Do not truncate the output until after checking whether it refers
+	 * to the input file.  Comparing the opened files also catches aliases
+	 * created with hard links or symbolic links.
+	 */
+	fout_fd = open(argv[optind + 1], O_WRONLY | O_CREAT, 0666);
+	if (fout_fd < 0) {
+		perror("open output");
+		exit(1);
+	}
+	if (fstat(fout_fd, &output_st)) {
+		perror("fstat output");
+		exit(1);
+	}
+	if (S_ISREG(st.st_mode) && S_ISREG(output_st.st_mode) &&
+	    st.st_dev == output_st.st_dev && st.st_ino == output_st.st_ino) {
+		fprintf(stderr, "Input and output files must be different\n");
+		exit(1);
+	}
+	if (S_ISREG(output_st.st_mode) && ftruncate(fout_fd, 0)) {
+		perror("truncate output");
+		exit(1);
+	}
+	fout = fdopen(fout_fd, "w");
+	if (!fout) {
+		perror("fdopen output");
+		exit(1);
+	}
+
 	if (!check_regcomp(&order_pattern, "order\\s*([0-9]*),"))
 		goto out_order;
 	if (!check_regcomp(&pid_pattern, "pid\\s*([0-9]*),"))
@@ -854,7 +888,6 @@ int main(int argc, char **argv)
 	if (!check_regcomp(&ts_nsec_pattern, "ts\\s*([0-9]*)\\s*ns"))
 		goto out_ts;
 
-	fstat(fileno(fin), &st);
 	max_size = st.st_size / 100; /* hack ... */
 
 	list = malloc(max_size * sizeof(*list));
-- 
2.39.5

Re: [PATCH] tools/mm: prevent page_owner_sort from truncating input

[Andrew Morton]

On Wed, 29 Jul 2026 13:38:12 +0800 Warren Xiong <warren.xiong@ugreen.com> wrote:

> page_owner_sort opens the output file with "w" before reading the input.
> If both paths refer to the same file, this truncates the input and the
> tool silently processes zero records before returning success.

Don't do that then?  I mean, is there any expectation that this works?

hp2:/usr/src/25> ls -l x
-rw-rw-r-- 1 akpm akpm 55318 Jul 29 17:27 x
hp2:/usr/src/25> sort -o x x
hp2:/usr/src/25> ls -l x    
-rw-rw-r-- 1 akpm akpm 55318 Jul 29 17:27 x

Oh.  sort(1) does that.  I never knew.

> Open the output without truncating it and compare the device and inode
> numbers of the opened files. Reject matching regular files, then truncate
> regular output files only after the check. This also detects hard link and
> symbolic link aliases while preserving non-regular outputs such as
> /dev/stdout.

This all sounds complex.  How does sort(1) do it?  Write to a temporary
file then a rename?

For page_owner_sort I expect we could load all the input into memory
before opening the output file?

Re: [PATCH] tools/mm: prevent page_owner_sort from truncating input

[熊卧龙]

> This all sounds complex. How does sort(1) do it? Write to a temporary
> file then a rename?
>
> For page_owner_sort I expect we could load all the input into memory
> before opening the output file?

I checked the GNU sort documentation. In normal mode, sort reads all
input before opening the output file. page_owner_sort already stores all
input records in memory, so moving fopen() after the input loop is simpler
and also allows in-place sorting.

I will rework the patch and send v2. Thanks for the suggestion.

Warren

[PATCH v2] tools/mm: prevent page_owner_sort from truncating input

[Warren Xiong]

page_owner_sort opens the output file with "w" before reading the input.
If both paths refer to the same file, this truncates the input and the
tool silently processes zero records before returning success.

Delay opening the output file until all input records have been loaded
into memory. This allows the tool to sort a file in place without
truncating data before it has been consumed.

Signed-off-by: Warren Xiong <warren.xiong@ugreen.com>
---
Changes in v2:
- Delay opening the output file until after all input has been loaded, as
  suggested by Andrew Morton.
- Allow in-place sorting instead of rejecting matching input and output
  files.

Tested with distinct paths, the same path, a hard-link alias, and a
symbolic-link alias.

Link to v1: https://lore.kernel.org/r/20260729053812.882137-1-warren.xiong@ugreen.com

 tools/mm/page_owner_sort.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index 35d3d2549..22b3b500d 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -836,8 +836,7 @@ int main(int argc, char **argv)
 	}
 
 	fin = fopen(argv[optind], "r");
-	fout = fopen(argv[optind + 1], "w");
-	if (!fin || !fout) {
+	if (!fin) {
 		usage();
 		perror("open: ");
 		exit(1);
@@ -874,6 +873,13 @@ int main(int argc, char **argv)
 			goto out_free;
 	}
 
+	fout = fopen(argv[optind + 1], "w");
+	if (!fout) {
+		usage();
+		perror("open: ");
+		exit(1);
+	}
+
 	printf("loaded %d\n", list_size);
 
 	printf("sorting ....\n");
-- 
2.39.5

Re: [PATCH v2] tools/mm: prevent page_owner_sort from truncating input

[Andrew Morton]

On Thu, 30 Jul 2026 09:58:09 +0800 Warren Xiong <warren.xiong@ugreen.com> wrote:

> page_owner_sort opens the output file with "w" before reading the input.
> If both paths refer to the same file, this truncates the input and the
> tool silently processes zero records before returning success.
> 
> Delay opening the output file until all input records have been loaded
> into memory. This allows the tool to sort a file in place without
> truncating data before it has been consumed.

Far nicer, thanks.

However I spotted a thing which Sashiko missed!

> @@ -874,6 +873,13 @@ int main(int argc, char **argv)
>  			goto out_free;
>  	}
>  
> +	fout = fopen(argv[optind + 1], "w");
> +	if (!fout) {
> +		usage();
> +		perror("open: ");
> +		exit(1);
> +	}

usage() calls printf(), which can set errno.  So perror() might print
the wrong thing.

Not that I care very much ;)