From: "Theo de Raadt" Subject: Re: Improve install -s To: Mark Kettenis Cc: tech@openbsd.org Date: Sat, 22 Aug 2026 19:56:58 -0600 kettenis' version made -s failures fatal. That does not match tradition. Originally the file was put into place and then strip was run to "attempt" to make symbol table reduction changes. So if strip fails, shrug, the complete file is ready for use and install would exit 0. Since the new code runs strip -o, we cannot be exactly certain of the outcome and have to do it carefully and correctly ourselves. Since it is just a request, -s is often used accidentally on all sorts of non-binary files. After a whole day of code iterations, I've have arrived at this version. If strip fails, some cleanup happens and the body of the install() function is-rerun without the strip request which will cause it to do the regular copy operations. Index: usr.bin/xinstall/install.1 =================================================================== RCS file: /cvs/src/usr.bin/xinstall/install.1,v diff -u -p -u -r1.31 install.1 --- usr.bin/xinstall/install.1 8 Feb 2019 12:53:44 -0000 1.31 +++ usr.bin/xinstall/install.1 22 Aug 2026 19:15:56 -0000 @@ -147,11 +147,13 @@ in the destination directory, then atomi This avoids both race conditions and the destruction of existing files in case of write failures. .It Fl s -.Nm -exec's the command +Attempt to strip the file of unneccessary sections and symbols. +This executes the command .Pa /usr/bin/strip -to strip binaries so that install can be portable over a large -number of systems and binary types. +using the +.Fl o +option to produce the output file, and if that fails copies the +file in the regular way. If the environment variable .Ev STRIP is set, it is used instead. @@ -175,7 +177,9 @@ creates an empty file. .It Ev STRIP For an alternate .Xr strip 1 -program to run. +program to run, which must support the +.Fl o +option. Default is .Pa /usr/bin/strip . .El Index: usr.bin/xinstall/xinstall.c =================================================================== RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v diff -u -p -u -r1.78 xinstall.c --- usr.bin/xinstall/xinstall.c 17 Oct 2024 15:38:38 -0000 1.78 +++ usr.bin/xinstall/xinstall.c 22 Aug 2026 22:41:39 -0000 @@ -72,7 +72,7 @@ void copy(int, char *, int, char *, off_ int compare(int, const char *, off_t, int, const char *, off_t); void install(char *, char *, u_long, u_int); void install_dir(char *, int); -void strip(char *); +int strip(char *, char *); void usage(void); int create_tempfile(char *, char *, size_t); int file_write(int, char *, size_t, int *, int *, int); @@ -220,7 +220,8 @@ install(char *from_name, char *to_name, { struct stat from_sb, to_sb; struct timespec ts[2]; - int devnull, from_fd, to_fd, serrno, files_match = 0; + int devnull, from_fd = -1, to_fd = -1, serrno, files_match = 0; + int l_dostrip = dostrip, l_docompare = docompare; char *p; char *target_name = tempfile; @@ -247,44 +248,61 @@ install(char *from_name, char *to_name, if (stat(to_name, &to_sb) == 0) { /* Only compare against regular files. */ - if (docompare && !S_ISREG(to_sb.st_mode)) { - docompare = 0; + if (l_docompare && !S_ISREG(to_sb.st_mode)) { + l_docompare = 0; warnc(EFTYPE, "%s", to_name); } - } else if (docompare) { + } else if (l_docompare) { /* File does not exist so silently ignore compare flag. */ - docompare = 0; + l_docompare = 0; } - if (!devnull) { +again: + if (!devnull && !l_dostrip) { if ((from_fd = open(from_name, O_RDONLY)) == -1) err(1, "%s", from_name); } to_fd = create_tempfile(to_name, tempfile, sizeof(tempfile)); if (to_fd < 0) - err(1, "%s", tempfile); + err(1, "mkstemp %s", tempfile); - if (!devnull) + if (!devnull && !l_dostrip) copy(from_fd, from_name, to_fd, tempfile, from_sb.st_size, ((off_t)from_sb.st_blocks * S_BLKSIZE < from_sb.st_size)); - if (dostrip) { - strip(tempfile); - - /* - * Re-open our fd on the target, in case we used a strip - * that does not work in-place -- like gnu binutils strip. - */ - close(to_fd); - if ((to_fd = open(tempfile, O_RDONLY)) == -1) - err(1, "stripping %s", to_name); + if (l_dostrip) { + if (strip(from_name, tempfile) == -1) { + /* + * Don't know why it failed. Input file does not + * exist? Not a strippable binary? Out of disk + * space? An error message has been issued, but + * -s is a request to attempt stripping of the + * resulting file. Try again without the strip attempt. + */ + if (from_fd != -1) + close(from_fd); + close(to_fd); + (void)unlink(tempfile); + l_dostrip = 0; + goto again; + } else { + /* + * strip(1) may have created a new file after + * unlink(2) or used rename(2) of a different + * file, so we must re-open. This is an ugly + * race against a long mktemp filename. + */ + close(to_fd); + if ((to_fd = open(tempfile, O_RDONLY)) == -1) + err(1, "%s", to_name); + } } /* * Compare the (possibly stripped) temp file to the target. */ - if (docompare) { + if (l_docompare) { int temp_fd = to_fd; struct stat temp_sb; @@ -531,8 +549,8 @@ compare(int from_fd, const char *from_na * strip -- * use strip(1) to strip the target file */ -void -strip(char *to_name) +int +strip(char *from_name, char *to_name) { int serrno, status; char * volatile path_strip; @@ -547,7 +565,8 @@ strip(char *to_name) (void)unlink(to_name); errc(1, serrno, "forks"); case 0: - execl(path_strip, "strip", "--", to_name, (char *)NULL); + execl(path_strip, "strip", "-o", to_name, "--", from_name, + (char *)NULL); warn("%s", path_strip); _exit(1); default: @@ -555,9 +574,10 @@ strip(char *to_name) if (errno != EINTR) break; } - if (!WIFEXITED(status)) - (void)unlink(to_name); + if (!WIFEXITED(status) || WEXITSTATUS(status)) + return -1; } + return 0; } /*