[gentoo-commits] proj/sandbox:master commit in: tests/, libsandbox/

["Mike Frysinger" <vapier@gentoo.org>]

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)