Hi Michal,
On Tue, Apr 1, 2025 at 2:56 PM Michal Sekletar <msekl...@redhat.com> wrote:
>
> Whenever possible, resolve all symlinks as if the sysroot path were a
> chroot environment. This prevents potential interactions with files from
> the host filesystem.
>
> Signed-off-by: Michal Sekletar <msekl...@redhat.com>
> ---
> configure.ac | 26 ++++++++++++++++++
> libdwfl/dwfl_segment_report_module.c | 39 +++++++++++++++++++++------
> libdwfl/link_map.c | 32 +++++++++++++++++++++-
> tests/run-sysroot.sh | 32 ++++++++++++++++++++++
> tests/testfile-sysroot-link.tar.bz2 | Bin 0 -> 2334030 bytes
> 5 files changed, 120 insertions(+), 9 deletions(-)
> create mode 100644 tests/testfile-sysroot-link.tar.bz2
>
> diff --git a/configure.ac b/configure.ac
> index 1679a17d..27ea7153 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -283,6 +283,32 @@ case "$CFLAGS" in
> ;;
> esac
>
> +AC_CACHE_CHECK(
> + [for openat2 with RESOLVE_IN_ROOT support],
> + [eu_cv_openat2_RESOLVE_IN_ROOT],
> + [AC_LINK_IFELSE(
> + [AC_LANG_PROGRAM(
> + [[#define _GNU_SOURCE
> + #include <fcntl.h>
> + #include <stdlib.h>
> + #include <unistd.h>
> + #include <sys/syscall.h>
> + #include <linux/openat2.h>
> + #include <stdio.h>
> + ]], [[
> + struct open_how how = { .flags = O_RDONLY|O_DIRECTORY, .resolve =
> RESOLVE_IN_ROOT };
> + int dfd = open (".", O_PATH);
> + return syscall (SYS_openat2, dfd, ".", &how, sizeof(how)) < 0;
> + ]]
> + )],
> + [eu_cv_openat2_RESOLVE_IN_ROOT=yes],
> + [eu_cv_openat2_RESOLVE_IN_ROOT=no]
> + )]
> +)
> +AS_IF([test "x$eu_cv_openat2_RESOLVE_IN_ROOT" = xyes],
> + [AC_DEFINE([HAVE_OPENAT2_RESOLVE_IN_ROOT], [1], [Define if openat2 is
> available])]
> +)
> +
> dnl enable debugging of branch prediction.
> AC_ARG_ENABLE([debugpred],
> AS_HELP_STRING([--enable-debugpred],[build binaries with support to debug
> branch prediction]),
> diff --git a/libdwfl/dwfl_segment_report_module.c
> b/libdwfl/dwfl_segment_report_module.c
> index 32f44af8..ee723114 100644
> --- a/libdwfl/dwfl_segment_report_module.c
> +++ b/libdwfl/dwfl_segment_report_module.c
> @@ -37,6 +37,12 @@
> #include <inttypes.h>
> #include <fcntl.h>
>
> +#if HAVE_OPENAT2_RESOLVE_IN_ROOT
> +#include <linux/openat2.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +#endif
> +
> #include <system.h>
>
>
> @@ -784,18 +790,35 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
> char *name,
> sysroot if it is set. */
> if (dwfl->sysroot && !executable)
> {
> - int r;
> - char *n;
> +#ifdef HAVE_OPENAT2_RESOLVE_IN_ROOT
> + int sysrootfd;
> + struct open_how how = {
> + .flags = O_RDONLY,
> + .resolve = RESOLVE_IN_ROOT,
> + };
> +
> + sysrootfd = open (dwfl->sysroot, O_DIRECTORY|O_PATH);
> + if (sysrootfd < 0)
> + return -1;
>
> - r = asprintf (&n, "%s%s", dwfl->sysroot, name);
> - if (r > 0)
> + fd = syscall (SYS_openat2, sysrootfd, name, &how, sizeof(how));
> + close (sysrootfd);
> +
> + if (fd < 0 && errno == ENOSYS)
> +#endif
> {
> - fd = open (n, O_RDONLY);
> - free (n);
> + int r;
> + char *n;
> + r = asprintf (&n, "%s%s", dwfl->sysroot, name);
> + if (r > 0)
> + {
> + fd = open (n, O_RDONLY);
> + free (n);
> + }
> }
> - }
> + }
> else
> - fd = open (name, O_RDONLY);
> + fd = open (name, O_RDONLY);
>
> if (fd >= 0)
> {
> diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
> index 8ab14862..c52ed041 100644
> --- a/libdwfl/link_map.c
> +++ b/libdwfl/link_map.c
> @@ -34,6 +34,12 @@
>
> #include <fcntl.h>
>
> +#if HAVE_OPENAT2_RESOLVE_IN_ROOT
> +#include <linux/openat2.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +#endif
> +
> /* This element is always provided and always has a constant value.
> This makes it an easy thing to scan for to discern the format. */
> #define PROBE_TYPE AT_PHENT
> @@ -418,7 +424,30 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
> elfdata,
> /* This code is mostly inlined dwfl_report_elf. */
> char *sysroot_name = NULL;
> const char *sysroot = dwfl->sysroot;
> + int fd;
> +
> +#ifdef HAVE_OPENAT2_RESOLVE_IN_ROOT
> + if (sysroot)
> + {
> + int sysrootfd;
> +
> + struct open_how how = {
> + .flags = O_RDONLY,
> + .resolve = RESOLVE_IN_ROOT,
> + };
>
> + sysrootfd = open (sysroot, O_DIRECTORY|O_PATH);
> + if (sysrootfd < 0)
> + return -1;
> +
> + fd = syscall (SYS_openat2, sysrootfd, name, &how, sizeof(how));
> + close (sysrootfd);
> +
> + /* Fallback to regular open() is openat2 is not available. */
> + if (fd < 0 && errno == ENOSYS)
> + fd = open (name, O_RDONLY);
> + }
If sysroot is NULL then fd is left uninitialized which may trigger
undefined behaviour below. We still should try to open the file and
set fd if sysroot is NULL, as is done when
HAVE_OPENAT2_RESOLVE_IN_ROOT isn't defined.
> +#else
> /* Don't use the sysroot if the path is already inside it. */
> bool name_in_sysroot = sysroot && startswith (name, sysroot);
>
> @@ -430,7 +459,8 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
> elfdata,
> name = sysroot_name;
> }
>
> - int fd = open (name, O_RDONLY);
> + fd = open (name, O_RDONLY);
> +#endif
> if (fd >= 0)
> {
> Elf *elf;
>diff --git a/tests/run-sysroot.sh b/tests/run-sysroot.sh
>index 1dc079cd..58415d8b 100755
>--- a/tests/run-sysroot.sh
>+++ b/tests/run-sysroot.sh
> @@ -46,4 +46,36 @@ TID 431185:
> #8 0x0000aaaae56127f0 _start
> EOF
> +HAVE_OPENAT2=$(grep '^#define HAVE_OPENAT2_RESOLVE_IN_ROOT' \
> + ${abs_builddir}/../config.h | awk '{print $3}')
> +
> +if [[ "$HAVE_OPENAT2" = 1 ]]; then
> + tmpdir1="$(mktemp -d)"
> +
> + tar xjf "${abs_srcdir}/testfile-sysroot-link.tar.bz2" -C "${tmpdir1}"
> +
> + # Check that stack with --sysroot generates correct backtrace even if
> target
> + # binary is actually absolute symlink pointing outside of sysroot
> directory
> + testrun "${abs_top_builddir}"/src/stack --core "${tmpdir1}/core.bash" \
> + --sysroot "${tmpdir1}/sysroot" >"${tmpdir1}/stack.out"
> +
> + # Remove 2 stack frames with symbol names contained in .gnu_debugdata.
> + # Whether or not these names appear in the output depends on if elfutils
> + # was built with LZMA support.
> + sed -i '4,5d' "${tmpdir1}/stack.out"
> +
> + # Check that we are able to get fully symbolized backtrace
> + testrun_compare cat "${tmpdir1}/stack.out" <<\EOF
> +PID 431185 - core
> +TID 431185:
> +#0 0x0000ffff8ebe5a8c kill
> +#3 0x0000aaaae562b2fc execute_command
> +#4 0x0000aaaae561cbb4 reader_loop
> +#5 0x0000aaaae5611bf0 main
> +#6 0x0000ffff8ebd09dc __libc_start_call_main
> +#7 0x0000ffff8ebd0ab0 __libc_start_main@@GLIBC_2.34
> +#8 0x0000aaaae56127f0 _start
> +EOF
> +fi
Thanks for adding these testcases. I have one nitpick, instead of
adding a whole new testfile-sysroot-link.tar.bz2 can you just add the
needed symlinks to testfile-sysroot.tar.bz2? With these changes plus
those recommended by Dmitry this patch will be ready for merging and
will make it into the 0.193 release.
Aaron
