diff options
| author |   Mike Frysinger <vapier@gentoo.org> | 2015-09-28 16:01:33 -0400 |
|---|---|---|
| committer | Mike Frysinger <vapier@gentoo.org> | 2015-09-28 16:01:33 -0400 |
| commit | 4377a68df2a20cda06aadb58c179ce2e8d78f7cd (patch) | |
| tree | e7645b4f4e950c9ee38186155975c755b8cbe190 | |
| parent | tests: add basic parsing of timespec fields (diff) | |
| download | sandbox-4377a68df2a20cda06aadb58c179ce2e8d78f7cd.tar.gz sandbox-4377a68df2a20cda06aadb58c179ce2e8d78f7cd.tar.bz2 sandbox-4377a68df2a20cda06aadb58c179ce2e8d78f7cd.zip | |
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@gentoo.org>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
| -rw-r--r-- | libsandbox/libsandbox.c | 15 | ||||
| -rwxr-xr-x | tests/utimensat-4.sh | 30 | ||||
| -rw-r--r-- | 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) |