Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Feb 2026 06:15:36 +0000
From:      Colin Percival <cperciva@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Cc:        Dag-Erling=?utf-8?Q? Sm=C3=B8rg?=rav <des@FreeBSD.org>
Subject:   git: 6761e555376e - releng/14.4 - diff: Fix pagination leak
Message-ID:  <698c1e88.34a1d.245c14d2@gitrepo.freebsd.org>

index | next in thread | raw e-mail

The branch releng/14.4 has been updated by cperciva:

URL: https://cgit.FreeBSD.org/src/commit/?id=6761e555376e375f89fe36c72f2813f5e64a7f63

commit 6761e555376e375f89fe36c72f2813f5e64a7f63
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2026-02-05 14:39:53 +0000
Commit:     Colin Percival <cperciva@FreeBSD.org>
CommitDate: 2026-02-11 06:14:26 +0000

    diff: Fix pagination leak
    
    * Drop an unnecessary variable and rename pidfd to procd.
    
    * Rewinding stdout serves no purpose, so stop doing it.
    
    * Don't bother freeing memory or setting the global status right
      before erroring out.
    
    * Error out if dup(2) or dup2(2) fail.
    
    * In the unlikely case that our pipe is equal to stdout, we need to
      record that information so we don't close it when cleaning up.
    
    * Don't bother closing a descriptor before dup2(2)ing to it.
    
    * Don't forget to close the the process descriptor after reaping the
      child process.
    
    Approved by:    re (cperciva)
    PR:             266592
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    kevans, markj
    Differential Revision:  https://reviews.freebsd.org/D55112
    
    (cherry picked from commit c3904a7de78ca1ca15fcdf4c09f9d4be7f6fe6f5)
    (cherry picked from commit 144455c333dc0d3db369596038de2e3dd6113b46)
---
 usr.bin/diff/pr.c               | 22 ++++++++++------------
 usr.bin/diff/pr.h               |  1 +
 usr.bin/diff/tests/diff_test.sh |  1 -
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/usr.bin/diff/pr.c b/usr.bin/diff/pr.c
index 5dedf689351c..20869e494d4a 100644
--- a/usr.bin/diff/pr.c
+++ b/usr.bin/diff/pr.c
@@ -45,7 +45,6 @@ struct pr *
 start_pr(char *file1, char *file2)
 {
 	int pfd[2];
-	int pr_pd;
 	pid_t pid;
 	char *header;
 	struct pr *pr;
@@ -55,13 +54,10 @@ start_pr(char *file1, char *file2)
 	xasprintf(&header, "%s %s %s", diffargs, file1, file2);
 	signal(SIGPIPE, SIG_IGN);
 	fflush(stdout);
-	rewind(stdout);
 	if (pipe(pfd) == -1)
 		err(2, "pipe");
-	switch ((pid = pdfork(&pr_pd, PD_CLOEXEC))) {
+	switch ((pid = pdfork(&pr->procd, PD_CLOEXEC))) {
 	case -1:
-		status |= 2;
-		free(header);
 		err(2, "No more processes");
 	case 0:
 		/* child */
@@ -73,21 +69,23 @@ start_pr(char *file1, char *file2)
 		execl(_PATH_PR, _PATH_PR, "-h", header, (char *)0);
 		_exit(127);
 	default:
-
 		/* parent */
-		if (pfd[1] != STDOUT_FILENO) {
-			pr->ostdout = dup(STDOUT_FILENO);
-			dup2(pfd[1], STDOUT_FILENO);
+		if (pfd[1] == STDOUT_FILENO) {
+			pr->ostdout = STDOUT_FILENO;
+		} else {
+			if ((pr->ostdout = dup(STDOUT_FILENO)) < 0 ||
+			    dup2(pfd[1], STDOUT_FILENO) < 0) {
+				err(2, "stdout");
+			}
 			close(pfd[1]);
 		}
 		close(pfd[0]);
-		rewind(stdout);
 		free(header);
 		pr->kq = kqueue();
 		if (pr->kq == -1)
 			err(2, "kqueue");
 		pr->e = xmalloc(sizeof(struct kevent));
-		EV_SET(pr->e, pr_pd, EVFILT_PROCDESC, EV_ADD, NOTE_EXIT, 0,
+		EV_SET(pr->e, pr->procd, EVFILT_PROCDESC, EV_ADD, NOTE_EXIT, 0,
 		    NULL);
 		if (kevent(pr->kq, pr->e, 1, NULL, 0, NULL) == -1)
 			err(2, "kevent");
@@ -106,7 +104,6 @@ stop_pr(struct pr *pr)
 
 	fflush(stdout);
 	if (pr->ostdout != STDOUT_FILENO) {
-		close(STDOUT_FILENO);
 		dup2(pr->ostdout, STDOUT_FILENO);
 		close(pr->ostdout);
 	}
@@ -114,6 +111,7 @@ stop_pr(struct pr *pr)
 		err(2, "kevent");
 	wstatus = pr->e[0].data;
 	close(pr->kq);
+	close(pr->procd);
 	free(pr);
 	if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != 0)
 		errx(2, "pr exited abnormally");
diff --git a/usr.bin/diff/pr.h b/usr.bin/diff/pr.h
index 2ff5949f282f..0a1275cfeab5 100644
--- a/usr.bin/diff/pr.h
+++ b/usr.bin/diff/pr.h
@@ -28,6 +28,7 @@
 
 struct pr {
 	int ostdout;
+	int procd;
 	int kq;
 	struct kevent *e;
 };
diff --git a/usr.bin/diff/tests/diff_test.sh b/usr.bin/diff/tests/diff_test.sh
index 2a07535d3d86..b499bb709cdd 100755
--- a/usr.bin/diff/tests/diff_test.sh
+++ b/usr.bin/diff/tests/diff_test.sh
@@ -421,7 +421,6 @@ prleak_body()
 	ulimit -n 1000
 	ulimit -u 1000
 	atf_check diff -rul a b
-	atf_check diff -Astone -rul a b
 }
 
 atf_init_test_cases()


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?698c1e88.34a1d.245c14d2>