git: 0ed86bcc0b2a - stable/13 - xinstall: fix dounpriv logic, add tests

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Wed, 02 Nov 2022 12:35:46 UTC
The branch stable/13 has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=0ed86bcc0b2adca5aad1446720799f6fdddb8dd7

commit 0ed86bcc0b2adca5aad1446720799f6fdddb8dd7
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2022-08-03 18:59:28 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2022-11-02 09:36:19 +0000

    xinstall: fix dounpriv logic, add tests
    
    Sponsored by:   Klara, Inc.
    MFC after:      1 week
    
    (cherry picked from commit 36d67475f5497664d33c41c2f6745dcb30b0ec42)
    
    xinstall: make md5 and ripemd160 conditional
    
    Sponsored by:   Klara, Inc.
    
    (cherry picked from commit c7a8e8d372b212c97dde6ce2731db27aa0b2201c)
    
    xinstall: use dynamic bufsize as in cat(1) / cp(1).
    
    Sponsored by:   Klara, Inc.
    
    (cherry picked from commit 54d8d0fe12a4996427923048ab4261819774fbd4)
    
    xinstall: alphabetize: upper case precedes lower.
    
    Sponsored by:   Klara, Inc.
    
    (cherry picked from commit f44e2577120c60cd92f685037bbb63127e0091e4)
---
 usr.bin/xinstall/Makefile              |   3 +-
 usr.bin/xinstall/install.1             |  14 ++---
 usr.bin/xinstall/tests/install_test.sh |  80 +++++++++++++++++++++++++
 usr.bin/xinstall/xinstall.c            | 105 +++++++++++++++++++++++++++++----
 4 files changed, 181 insertions(+), 21 deletions(-)

diff --git a/usr.bin/xinstall/Makefile b/usr.bin/xinstall/Makefile
index ce70cb882190..9969ef104e98 100644
--- a/usr.bin/xinstall/Makefile
+++ b/usr.bin/xinstall/Makefile
@@ -14,7 +14,8 @@ MAN=		install.1
 CFLAGS+=	-I${SRCTOP}/contrib/mtree
 CFLAGS+=	-I${SRCTOP}/lib/libnetbsd
 
-LIBADD=	md
+LIBADD=		md
+CFLAGS+=	-DWITH_MD5 -DWITH_RIPEMD160
 
 HAS_TESTS=
 SUBDIR.${MK_TESTS}+= tests
diff --git a/usr.bin/xinstall/install.1 b/usr.bin/xinstall/install.1
index 4d4db9633d9b..2f1bb080d9c0 100644
--- a/usr.bin/xinstall/install.1
+++ b/usr.bin/xinstall/install.1
@@ -28,7 +28,7 @@
 .\"	From: @(#)install.1	8.1 (Berkeley) 6/6/93
 .\" $FreeBSD$
 .\"
-.Dd August 12, 2019
+.Dd August 4, 2022
 .Dt INSTALL 1
 .Os
 .Sh NAME
@@ -99,6 +99,12 @@ option's argument.
 .Pp
 The options are as follows:
 .Bl -tag -width indent
+.It Fl B Ar suffix
+Use
+.Ar suffix
+as the backup suffix if
+.Fl b
+is given.
 .It Fl b
 Back up any existing files before overwriting them by renaming
 them to
@@ -106,12 +112,6 @@ them to
 See
 .Fl B
 for specifying a different backup suffix.
-.It Fl B Ar suffix
-Use
-.Ar suffix
-as the backup suffix if
-.Fl b
-is given.
 .It Fl C
 Copy the file.
 If the target file already exists and the files are the same,
diff --git a/usr.bin/xinstall/tests/install_test.sh b/usr.bin/xinstall/tests/install_test.sh
index c723a6e0d280..e95f40fc19e7 100755
--- a/usr.bin/xinstall/tests/install_test.sh
+++ b/usr.bin/xinstall/tests/install_test.sh
@@ -404,6 +404,84 @@ symbolic_link_relative_absolute_common_body() {
 	fi
 }
 
+atf_test_case set_owner_group_mode
+set_owner_group_mode_head() {
+	atf_set "require.user" "root"
+}
+set_owner_group_mode_body() {
+	local fu=65531 fg=65531
+	local cu=65532 cg=65532
+	local u="$(id -u)"
+	local g="$(id -g)"
+	local m=0755 cm=4444
+	printf "test" >testf
+	atf_check chown "$fu:$fg" testf
+	atf_check chmod "$m" testf
+
+	atf_check install testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" testf testc
+	atf_check_equal "$cu:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -g "$cg" testf testc
+	atf_check_equal "$u:$cg:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" -g "$cg" testf testc
+	atf_check_equal "$cu:$cg:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cm" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" -m "$cm" testf testc
+	atf_check_equal "$cu:$g:10$cm" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$u:$cg:10$cm" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -o "$cu" -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$cu:$cg:10$cm" "$(stat -f"%u:%g:%p" testc)"
+}
+
+atf_test_case set_owner_group_mode_unpriv
+set_owner_group_mode_unpriv_head() {
+	atf_set "require.user" "root"
+}
+set_owner_group_mode_unpriv_body() {
+	local fu=65531 fg=65531
+	local cu=65532 cg=65532
+	local u="$(id -u)"
+	local g="$(id -g)"
+	local m=0755 cm=4444 cM=0444
+	printf "test" >testf
+	atf_check chown "$fu:$fg" testf
+	atf_check chmod "$m" testf
+
+	atf_check install -U testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -g "$cg" testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" -g "$cg" testf testc
+	atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+
+	atf_check install -U -o "$cu" -g "$cg" -m "$cm" testf testc
+	atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)"
+}
+
 atf_init_test_cases() {
 	atf_add_test_case copy_to_nonexistent
 	atf_add_test_case copy_to_nonexistent_safe
@@ -444,4 +522,6 @@ atf_init_test_cases() {
 	atf_add_test_case symbolic_link_relative_absolute_source_and_dest2
 	atf_add_test_case symbolic_link_relative_absolute_common
 	atf_add_test_case mkdir_simple
+	atf_add_test_case set_owner_group_mode
+	atf_add_test_case set_owner_group_mode_unpriv
 }
diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c
index 114614abd16f..691a16191bba 100644
--- a/usr.bin/xinstall/xinstall.c
+++ b/usr.bin/xinstall/xinstall.c
@@ -57,10 +57,14 @@ __FBSDID("$FreeBSD$");
 #include <fcntl.h>
 #include <grp.h>
 #include <libgen.h>
+#ifdef WITH_MD5
 #include <md5.h>
+#endif
 #include <paths.h>
 #include <pwd.h>
+#ifdef WITH_RIPEMD160
 #include <ripemd.h>
+#endif
 #include <sha.h>
 #include <sha256.h>
 #include <sha512.h>
@@ -75,6 +79,21 @@ __FBSDID("$FreeBSD$");
 
 #include "mtree.h"
 
+/*
+ * Memory strategy threshold, in pages: if physmem is larger then this, use a
+ * large buffer.
+ */
+#define PHYSPAGES_THRESHOLD (32*1024)
+
+/* Maximum buffer size in bytes - do not allow it to grow larger than this. */
+#define BUFSIZE_MAX (2*1024*1024)
+
+/*
+ * Small (default) buffer size in bytes. It's inefficient for this to be
+ * smaller than MAXPHYS.
+ */
+#define BUFSIZE_SMALL (MAXPHYS)
+
 /*
  * We need to build xinstall during the bootstrap stage when building on a
  * non-FreeBSD system. Linux does not have the st_flags and st_birthtime
@@ -100,8 +119,12 @@ __FBSDID("$FreeBSD$");
 #define	BACKUP_SUFFIX	".old"
 
 typedef union {
+#ifdef WITH_MD5
 	MD5_CTX		MD5;
+#endif
+#ifdef WITH_RIPEMD160
 	RIPEMD160_CTX	RIPEMD160;
+#endif
 	SHA1_CTX	SHA1;
 	SHA256_CTX	SHA256;
 	SHA512_CTX	SHA512;
@@ -109,8 +132,12 @@ typedef union {
 
 static enum {
 	DIGEST_NONE = 0,
+#ifdef WITH_MD5
 	DIGEST_MD5,
+#endif
+#ifdef WITH_RIPEMD160
 	DIGEST_RIPEMD160,
+#endif
 	DIGEST_SHA1,
 	DIGEST_SHA256,
 	DIGEST_SHA512,
@@ -288,10 +315,14 @@ main(int argc, char *argv[])
 	if (digest != NULL) {
 		if (strcmp(digest, "none") == 0) {
 			digesttype = DIGEST_NONE;
+#ifdef WITH_MD5
 		} else if (strcmp(digest, "md5") == 0) {
 		       digesttype = DIGEST_MD5;
+#endif
+#ifdef WITH_RIPEMD160
 		} else if (strcmp(digest, "rmd160") == 0) {
 			digesttype = DIGEST_RIPEMD160;
+#endif
 		} else if (strcmp(digest, "sha1") == 0) {
 			digesttype = DIGEST_SHA1;
 		} else if (strcmp(digest, "sha256") == 0) {
@@ -402,10 +433,14 @@ digest_file(const char *name)
 {
 
 	switch (digesttype) {
+#ifdef WITH_MD5
 	case DIGEST_MD5:
 		return (MD5File(name, NULL));
+#endif
+#ifdef WITH_RIPEMD160
 	case DIGEST_RIPEMD160:
 		return (RIPEMD160_File(name, NULL));
+#endif
 	case DIGEST_SHA1:
 		return (SHA1_File(name, NULL));
 	case DIGEST_SHA256:
@@ -424,12 +459,16 @@ digest_init(DIGEST_CTX *c)
 	switch (digesttype) {
 	case DIGEST_NONE:
 		break;
+#ifdef WITH_MD5
 	case DIGEST_MD5:
 		MD5Init(&(c->MD5));
 		break;
+#endif
+#ifdef WITH_RIPEMD160
 	case DIGEST_RIPEMD160:
 		RIPEMD160_Init(&(c->RIPEMD160));
 		break;
+#endif
 	case DIGEST_SHA1:
 		SHA1_Init(&(c->SHA1));
 		break;
@@ -449,12 +488,16 @@ digest_update(DIGEST_CTX *c, const char *data, size_t len)
 	switch (digesttype) {
 	case DIGEST_NONE:
 		break;
+#ifdef WITH_MD5
 	case DIGEST_MD5:
 		MD5Update(&(c->MD5), data, len);
 		break;
+#endif
+#ifdef WITH_RIPEMD160
 	case DIGEST_RIPEMD160:
 		RIPEMD160_Update(&(c->RIPEMD160), data, len);
 		break;
+#endif
 	case DIGEST_SHA1:
 		SHA1_Update(&(c->SHA1), data, len);
 		break;
@@ -472,10 +515,14 @@ digest_end(DIGEST_CTX *c, char *buf)
 {
 
 	switch (digesttype) {
+#ifdef WITH_MD5
 	case DIGEST_MD5:
 		return (MD5End(&(c->MD5), buf));
+#endif
+#ifdef WITH_RIPEMD160
 	case DIGEST_RIPEMD160:
 		return (RIPEMD160_End(&(c->RIPEMD160), buf));
+#endif
 	case DIGEST_SHA1:
 		return (SHA1_End(&(c->SHA1), buf));
 	case DIGEST_SHA256:
@@ -1009,19 +1056,18 @@ install(const char *from_name, const char *to_name, u_long fset, u_int flags)
 #endif
 	}
 
-	if (!dounpriv & 
-	    (gid != (gid_t)-1 && gid != to_sb.st_gid) ||
-	    (uid != (uid_t)-1 && uid != to_sb.st_uid))
+	if (!dounpriv && ((gid != (gid_t)-1 && gid != to_sb.st_gid) ||
+	    (uid != (uid_t)-1 && uid != to_sb.st_uid))) {
 		if (fchown(to_fd, uid, gid) == -1) {
 			serrno = errno;
 			(void)unlink(to_name);
 			errno = serrno;
 			err(EX_OSERR,"%s: chown/chgrp", to_name);
 		}
-
+	}
 	if (mode != (to_sb.st_mode & ALLPERMS)) {
 		if (fchmod(to_fd,
-		     dounpriv ? mode & (S_IRWXU|S_IRWXG|S_IRWXO) : mode)) {
+		    dounpriv ? mode & (S_IRWXU|S_IRWXG|S_IRWXO) : mode)) {
 			serrno = errno;
 			(void)unlink(to_name);
 			errno = serrno;
@@ -1036,7 +1082,7 @@ install(const char *from_name, const char *to_name, u_long fset, u_int flags)
 	 * trying to turn off UF_NODUMP.  If we're trying to set real flags,
 	 * then warn if the fs doesn't support it, otherwise fail.
 	 */
-	if (!dounpriv & !devnull && (flags & SETFLAGS ||
+	if (!dounpriv && !devnull && (flags & SETFLAGS ||
 	    (from_sb.st_flags & ~UF_NODUMP) != to_sb.st_flags) &&
 	    fchflags(to_fd,
 	    flags & SETFLAGS ? fset : from_sb.st_flags & ~UF_NODUMP)) {
@@ -1082,7 +1128,7 @@ compare(int from_fd, const char *from_name __unused, size_t from_len,
 		return 1;
 
 	do_digest = (digesttype != DIGEST_NONE && dresp != NULL &&
-			*dresp == NULL);
+	    *dresp == NULL);
 	if (from_len <= MAX_CMP_SIZE) {
 		if (do_digest)
 			digest_init(&ctx);
@@ -1108,15 +1154,32 @@ compare(int from_fd, const char *from_name __unused, size_t from_len,
 		}
 	out:
 		if (!done_compare) {
-			char buf1[MAXBSIZE];
-			char buf2[MAXBSIZE];
+			static char *buf, *buf1, *buf2;
+			static size_t bufsize;
 			int n1, n2;
 
+			if (buf == NULL) {
+				/*
+				 * Note that buf and bufsize are static. If
+				 * malloc() fails, it will fail at the start
+				 * and not copy only some files.
+				 */
+				if (sysconf(_SC_PHYS_PAGES) >
+				    PHYSPAGES_THRESHOLD)
+					bufsize = MIN(BUFSIZE_MAX, MAXPHYS * 8);
+				else
+					bufsize = BUFSIZE_SMALL;
+				buf = malloc(bufsize * 2);
+				if (buf == NULL)
+					err(1, "Not enough memory");
+				buf1 = buf;
+				buf2 = buf + bufsize;
+			}
 			rv = 0;
 			lseek(from_fd, 0, SEEK_SET);
 			lseek(to_fd, 0, SEEK_SET);
 			while (rv == 0) {
-				n1 = read(from_fd, buf1, sizeof(buf1));
+				n1 = read(from_fd, buf1, bufsize);
 				if (n1 == 0)
 					break;		/* EOF */
 				else if (n1 > 0) {
@@ -1233,10 +1296,11 @@ static char *
 copy(int from_fd, const char *from_name, int to_fd, const char *to_name,
     off_t size)
 {
+	static char *buf = NULL;
+	static size_t bufsize;
 	int nr, nw;
 	int serrno;
 	char *p;
-	char buf[MAXBSIZE];
 	int done_copy;
 	DIGEST_CTX ctx;
 
@@ -1270,7 +1334,22 @@ copy(int from_fd, const char *from_name, int to_fd, const char *to_name,
 		done_copy = 1;
 	}
 	if (!done_copy) {
-		while ((nr = read(from_fd, buf, sizeof(buf))) > 0) {
+		if (buf == NULL) {
+			/*
+			 * Note that buf and bufsize are static. If
+			 * malloc() fails, it will fail at the start
+			 * and not copy only some files.
+			 */
+			if (sysconf(_SC_PHYS_PAGES) >
+			    PHYSPAGES_THRESHOLD)
+				bufsize = MIN(BUFSIZE_MAX, MAXPHYS * 8);
+			else
+				bufsize = BUFSIZE_SMALL;
+			buf = malloc(bufsize);
+			if (buf == NULL)
+				err(1, "Not enough memory");
+		}
+		while ((nr = read(from_fd, buf, bufsize)) > 0) {
 			if ((nw = write(to_fd, buf, nr)) != nr) {
 				serrno = errno;
 				(void)unlink(to_name);
@@ -1380,7 +1459,7 @@ install_dir(char *path)
 			ch = *p;
 			*p = '\0';
 again:
-			if (stat(path, &sb) < 0) {
+			if (stat(path, &sb) != 0) {
 				if (errno != ENOENT || tried_mkdir)
 					err(EX_OSERR, "stat %s", path);
 				if (mkdir(path, 0755) < 0) {