With -o elfcompress opens the output file with O_WRONLY and O_CREAT.
If the output file already existed then without O_TRUNC the file is
written from the start, but keeps all existing data. That means the
file might contain extra data if the (compressed) ELF file is shorter
than the existing file. Without -o the input file is the output file
and we create a temp file to create the output which should then be
atomically replaced using rename. This might fail when the input file
was a symlink (to a file in a different directory.)
Solve both issues by first trying to open the output file with
O_EXCL. Which makes sure we created the file so we can write directly
to it. If it fails then the output file already exists. Then we first
resolve the full file path (in case the output file is a symlink) and
then open the directory where the output should go. We then make sure
the create a temporary file inside this directory using a new system.h
helper function xmkstempat. Finally, after the output file is fully
created (and flushed out to disk) we use renameat to atomically
replace the output file with our temp file. Since we still have the
directory open this works even if the file path is changed while
creating the temp file data.
Add a new run-compress-test-out.sh testcase for input and/or output
file being symlinks and/or pointing to the same file.
* lib/system.h (xmkstempat): New helper function.
* src/elfcompress.c (process_file): Declare new variables to
keep copies of the (ouput) directory, dirfd, base file and
temporary file name. Fix trailing \n in error fmt. Call open
with O_EXCL for output file. Use realpath and xmkstempat if
output file exists. fsync temp file before caling
renameat. Use unlinkat in cleanup. close and free new
temporaries.
* tests/run-compress-test-out.sh: New test file.
* tests/Makefile.am (TESTS): Add new test file.
(EXTRA_DIST): Likewise.
Signed-off-by: Mark Wielaard <[email protected]>
---
lib/system.h | 52 ++++++++++++++-
src/elfcompress.c | 112 +++++++++++++++++++++++++--------
tests/Makefile.am | 4 +-
tests/run-compress-test-out.sh | 74 ++++++++++++++++++++++
4 files changed, 214 insertions(+), 28 deletions(-)
create mode 100755 tests/run-compress-test-out.sh
diff --git a/lib/system.h b/lib/system.h
index c17e2aa0fea1..51fc1f89130d 100644
--- a/lib/system.h
+++ b/lib/system.h
@@ -1,6 +1,6 @@
/* Declarations for common convenience functions.
Copyright (C) 2006-2011 Red Hat, Inc.
- Copyright (C) 2022 Mark J. Wielaard <[email protected]>
+ Copyright (C) 2022, 2026 Mark J. Wielaard <[email protected]>
Copyright (C) 2023 Khem Raj.
This file is part of elfutils.
@@ -39,6 +39,7 @@
#endif
#include <errno.h>
+#include <fcntl.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
@@ -52,6 +53,7 @@
#include <sys/mman.h>
#include <sys/param.h>
#include <unistd.h>
+#include <sys/random.h>
#if defined(HAVE_ERROR_H)
#include <error.h>
@@ -257,4 +259,52 @@ xbasename(const char *s)
}
#pragma GCC poison basename
+/* There is no mkstempat needed for creating a temp file in a specific
+ directory. Needed e.g. in combination with renameat to atomicly
+ replace a file. So define one ourselves. Like mkstemp the template
+ must end with must be "XXXXXX", which are replaced by an unique
+ https://sourceware.org/bugzilla/show_bug.cgi?id=19866 */
+static inline int
+xmkstempat (int dirfd, char *templ)
+{
+ /* Only use these 64 chars. */
+ const char chars[] =
+ "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-_";
+
+ /* Must end in 6X. */
+ size_t l = strlen (templ);
+ if (l < 6 || memcmp (templ + l - 6, "XXXXXX", 6) != 0)
+ {
+ errno = EINVAL;
+ return -1;
+ }
+
+ int tries = 128; /* Just fail with EEXIST if 128 tries wasn't enough. */
+ do
+ {
+ uint64_t r; /* We need at least 64^6 == 2^36 */
+ if (TEMP_FAILURE_RETRY (getrandom (&r, sizeof (r), 0)) != sizeof (r))
+ return -1;
+
+ /* Random chars for the template. */
+ for (int i = 0; i < 6; i++)
+ {
+ templ[l - 6 + i] = chars[r % 64];
+ r /= 64;
+ }
+
+ /* Must be able to open exclusively. */
+ int fd = openat (dirfd, templ,
+ O_RDWR | O_CREAT | O_EXCL,
+ S_IRUSR | S_IWUSR);
+ if (fd >= 0)
+ return fd;
+
+ tries--;
+ }
+ while (tries > 0 && errno == EEXIST);
+
+ return -1;
+}
+
#endif /* system.h */
diff --git a/src/elfcompress.c b/src/elfcompress.c
index fdcff32847ce..41cf9c10d0e6 100644
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -1,5 +1,6 @@
/* Compress or decompress an ELF file.
Copyright (C) 2015, 2016, 2018 Red Hat, Inc.
+ Copyright (C) 2026 Mark J. Wielaard <[email protected]>
This file is part of elfutils.
This file is free software; you can redistribute it and/or modify
@@ -37,6 +38,9 @@
#include "libeu.h"
#include "printversion.h"
+/* Really should come from libgen.h, but we poisoned basename in system.h. */
+extern char *dirname(char *path);
+
/* Name and version of program. */
ARGP_PROGRAM_VERSION_HOOK_DEF = print_version;
@@ -342,6 +346,14 @@ process_file (const char *fname)
int fdnew = -1;
Elf *elfnew = NULL;
+ /* Split up dir/base names if necessary. */
+ char *dirc = NULL;
+ char *basec = NULL;
+ const char *bname = NULL;
+ const char *dname = NULL;
+ int dirfd = -1;
+ char *tempname = NULL;
+
/* Buffer for (one) new section name if necessary. */
char *snamebuf = NULL;
@@ -370,7 +382,7 @@ process_file (const char *fname)
fd = open (fname, O_RDONLY);
if (fd < 0)
{
- error (0, errno, "Couldn't open %s\n", fname);
+ error (0, errno, "Couldn't open %s", fname);
goto cleanup;
}
@@ -611,26 +623,68 @@ process_file (const char *fname)
scnnames = xcalloc (shnum, sizeof (char *));
}
- /* Create a new (temporary) ELF file for the result. */
- if (foutput == NULL)
+ /* Now deal with the output. If we can (exclusively) open the
+ output file directly, we can just use that. */
+ fnew = xstrdup (foutput == NULL ? fname : foutput);
+ fdnew = open (fnew, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC,
+ st.st_mode & ALLPERMS);
+
+ /* If we cannot open the output exclusively for writing directly
+ (because it already exists, e.g. it might be the current input
+ file, then we want to write to a temporary file first and then
+ (atomically) replace it. This is slightly tricky. To make sure
+ the replacement (rename) is atomic the temp file and final file
+ need to be in the same directory. We use realpath to make sure
+ we end up in the actual directory that the output is in if it was
+ a symlink. To make sure the directory path doesn't change
+ between temp file creation and rename we need to keep a dirfd
+ open. */
+ if (fdnew < 0 && errno == EEXIST)
{
- size_t fname_len = strlen (fname);
- fnew = xmalloc (fname_len + sizeof (".XXXXXX"));
- strcpy (mempcpy (fnew, fname, fname_len), ".XXXXXX");
- fdnew = mkstemp (fnew);
- }
- else
- {
- fnew = xstrdup (foutput);
- fdnew = open (fnew, O_WRONLY | O_CREAT, st.st_mode & ALLPERMS);
+ /* OK, it already existed (or was a symlink). Try again, but now
+ with the resolved path. */
+ free (fnew);
+ fnew = realpath (foutput == NULL ? fname : foutput, NULL);
+ if (fnew == NULL)
+ {
+ error (0, errno, "Couldn't get realpath for %s",
+ foutput == NULL ? fname : foutput);
+ goto cleanup;
+ }
+
+ /* Split up the path into the dir and base parts. */
+ dirc = xstrdup (fnew);
+ dname = dirname (dirc);
+ basec = xstrdup (fnew);
+ bname = xbasename (basec);
+
+ /* Pin the directory. */
+ dirfd = open (dname, O_RDONLY | O_DIRECTORY);
+ if (dirfd < 0)
+ {
+ error (0, errno, "Couldn't open output dir %s", dname);
+ goto cleanup;
+ }
+
+ /* Create a temp file inside the output dir. This could
+ possibly done with O_TMPFILE and then using /proc/self/fd to
+ rename. But it is not clear how portable that is. */
+ size_t fnew_len = strlen (fnew);
+ tempname = xmalloc (fnew_len + sizeof (".XXXXXX"));
+ sprintf (tempname, "%s.XXXXXX", fnew);
+ fdnew = xmkstempat (dirfd, tempname);
}
if (fdnew < 0)
{
- error (0, errno, "Couldn't create output file %s", fnew);
+ error (0, errno, "Couldn't create output file %s",
+ tempname == NULL ? fnew : tempname);
/* Since we didn't create it we don't want to try to unlink it. */
- free (fnew);
- fnew = NULL;
+ if (tempname != NULL)
+ {
+ free (fnew);
+ fnew = NULL;
+ }
goto cleanup;
}
@@ -1358,12 +1412,15 @@ process_file (const char *fname)
error (0, errno, "Couldn't fchmod %s", fnew);
/* Finally replace the old file with the new file. */
- if (foutput == NULL)
- if (rename (fnew, fname) != 0)
- {
- error (0, errno, "Couldn't rename %s to %s", fnew, fname);
- goto cleanup;
- }
+ if (tempname != NULL)
+ {
+ fsync (fdnew); /* Flush all data and metadata before replacing file. */
+ if (renameat (dirfd, tempname, dirfd, bname) != 0)
+ {
+ error (0, errno, "Couldn't rename %s to %s", tempname, fnew);
+ goto cleanup;
+ }
+ }
/* We are finally done with the new file, don't unlink it now. */
free (fnew);
@@ -1377,13 +1434,18 @@ cleanup:
elf_end (elfnew);
close (fdnew);
- if (fnew != NULL)
+ if (tempname != NULL)
{
- unlink (fnew);
- free (fnew);
- fnew = NULL;
+ unlinkat (dirfd, tempname, 0);
+ free (tempname);
}
+ if (dirfd >= 0)
+ close (dirfd);
+
+ free (dirc);
+ free (basec);
+
free (snamebuf);
if (names != NULL)
{
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8ae7d126636e..fb0690edaf57 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -206,7 +206,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile
test-nlist \
elfshphehdr run-lfs-symbols.sh run-dwelfgnucompressed.sh \
run-elfgetchdr.sh \
run-elfgetzdata.sh run-elfputzdata.sh run-zstrptr.sh \
- run-compress-test.sh \
+ run-compress-test.sh run-compress-test-out.sh \
run-readelf-zdebug.sh run-readelf-zdebug-rel.sh \
emptyfile vendorelf fillfile dwarf_default_lower_bound \
dwarf_language_lower_bound \
@@ -575,7 +575,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
testfile-zgabi32.bz2 testfile-zgabi64.bz2 \
testfile-zgabi32be.bz2 testfile-zgabi64be.bz2 \
run-elfgetchdr.sh run-elfgetzdata.sh run-elfputzdata.sh \
- run-zstrptr.sh run-compress-test.sh \
+ run-zstrptr.sh run-compress-test.sh run-compress-test-out.sh \
run-disasm-bpf.sh \
testfile-bpf-dis1.expect.bz2 testfile-bpf-dis1.o.bz2 \
run-reloc-bpf.sh \
diff --git a/tests/run-compress-test-out.sh b/tests/run-compress-test-out.sh
new file mode 100755
index 000000000000..8e51e1c1ac7f
--- /dev/null
+++ b/tests/run-compress-test-out.sh
@@ -0,0 +1,74 @@
+#! /bin/sh
+# Copyright (C) 2026 Mark J. Wielaard <[email protected]>
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/test-subr.sh
+
+testrun_elfcompress_out()
+{
+ testfile="$1"
+ testfiles ${testfile}
+
+ # Direct compress
+ testrun ${abs_top_builddir}/src/elfcompress -v -t zlib-gnu ${testfile}
+ testrun ${abs_top_builddir}/src/elflint --gnu-ld ${testfile}
+
+ # Decompress with -o being the input file
+ testrun ${abs_top_builddir}/src/elfcompress -v -t none -o ${testfile} \
+ ${testfile}
+ testrun ${abs_top_builddir}/src/elflint --gnu-ld ${testfile}
+
+ # Compress with -o being an existing file
+ tempfiles ${testfile}.tmp
+ touch ${testfile}.tmp
+ testrun ${abs_top_builddir}/src/elfcompress -v -t zlib -o ${testfile}.tmp \
+ ${testfile}
+ testrun ${abs_top_builddir}/src/elflint --gnu-ld ${testfile}.tmp
+
+ # Decompress with -o being a symlink to the input
+ tempfiles ${testfile}.link
+ ln -s ${testfile}.tmp ${testfile}.link
+ testrun ${abs_top_builddir}/src/elfcompress -v -t none -o ${testfile}.link \
+ ${testfile}.tmp
+ testrun ${abs_top_builddir}/src/elflint --gnu-ld ${testfile}.link
+
+ # Compress with input being a symlink to a file in a nested directory
+ tempfiles ${testfile}.deep
+ mkdir deep
+ cp ${testfile} deep/
+ ln -s deep/${testfile} ${testfile}.deep
+ testrun ${abs_top_builddir}/src/elfcompress -v -t zlib ${testfile}.deep
+ testrun ${abs_top_builddir}/src/elflint --gnu-ld deep/${testfile}
+ rm deep/${testfile}
+ rmdir deep
+}
+
+# The actual test file shouldn't matter, but just use a couple of
+# different ones.
+
+# Random ELF32 testfile
+testrun_elfcompress_out testfile4
+
+# Random ELF64 testfile
+testrun_elfcompress_out testfile12
+
+# Random ELF64BE testfile
+testrun_elfcompress_out testfileppc64
+
+# Random ELF32BE testfile
+testrun_elfcompress_out testfileppc32
+
+exit 0
--
2.54.0
