commit: 4377a68df2a20cda06aadb58c179ce2e8d78f7cd Author: Mike Frysinger <vapier <AT> gentoo <DOT> org> AuthorDate: Mon Sep 28 20:01:33 2015 +0000 Commit: Mike Frysinger <vapier <AT> gentoo <DOT> org> CommitDate: Mon Sep 28 20:01:33 2015 +0000 URL: https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=4377a68d
libsandbox: do not unnecessarily dereference symlinks When the target uses a func that operates on a symlink, we should not dereference that symlink when trying to validate the call. It's both a waste of time and it subtly breaks code that checks atime updates. The act of reading symlinks is enough to cause their atime to change. URL: https://bugs.gentoo.org/415475 Reported-by: Marien Zwart <marienz <AT> gentoo.org> Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org> libsandbox/libsandbox.c | 15 ++++++++++++--- tests/utimensat-4.sh | 30 ++++++++++++++++++++++++++++++ tests/utimensat.at | 1 + 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c index 1d9fa04..2bcff95 100644 --- a/libsandbox/libsandbox.c +++ b/libsandbox/libsandbox.c @@ -909,7 +909,14 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func, bool access, debug, verbose, set; absolute_path = resolve_path(file, 0); - resolved_path = resolve_path(file, 1); + /* Do not bother dereferencing symlinks when we are using a function that + * itself does not dereference. This speeds things up and avoids updating + * the atime implicitly. #415475 + */ + if (symlink_func(sb_nr, flags, absolute_path)) + resolved_path = absolute_path; + else + resolved_path = resolve_path(file, 1); if (!absolute_path || !resolved_path) goto error; sb_debug_dyn("absolute_path: %s\n", absolute_path); @@ -955,7 +962,8 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func, } free(absolute_path); - free(resolved_path); + if (absolute_path != resolved_path) + free(resolved_path); errno = old_errno; @@ -967,7 +975,8 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func, */ if (errno_is_too_long()) { free(absolute_path); - free(resolved_path); + if (absolute_path != resolved_path) + free(resolved_path); return 2; } diff --git a/tests/utimensat-4.sh b/tests/utimensat-4.sh new file mode 100755 index 0000000..731c7d1 --- /dev/null +++ b/tests/utimensat-4.sh @@ -0,0 +1,30 @@ +#!/bin/sh +# make sure we don't accidentally trip atime updates on files +# through symlinks #415475 +[ "${at_xfail}" = "yes" ] && exit 77 # see script-0 + +# We assume $PWD supports atimes, and the granularity is more than 1 second. +# If it doesn't, this test will still pass, but not really because the code +# was proven to be correct. + +# XXX: Maybe we need to add our own stat shim to avoid portability issues ? +get_atime() { + # This shows the full atime field (secs, msecs, nsecs). + stat -c %x "$1" +} + +# Create a symlink. +sym="sym" +ln -s atime "${sym}" + +# Get the state before we test it. +before=$(get_atime "${sym}") + +# A quick sleep of a few msecs. +sleep 0.1 + +# See if the atime changes -- it should not. +utimensat-0 -1,EINVAL AT_FDCWD "${sym}" -1,-1 AT_SYMLINK_NOFOLLOW || exit 1 +after=$(get_atime "${sym}") + +[ "${after}" = "${before}" ] diff --git a/tests/utimensat.at b/tests/utimensat.at index eec4638..1909650 100644 --- a/tests/utimensat.at +++ b/tests/utimensat.at @@ -1,3 +1,4 @@ SB_CHECK(1) SB_CHECK(2) SB_CHECK(3) +SB_CHECK(4)
