1 |
commit: 4377a68df2a20cda06aadb58c179ce2e8d78f7cd |
2 |
Author: Mike Frysinger <vapier <AT> gentoo <DOT> org> |
3 |
AuthorDate: Mon Sep 28 20:01:33 2015 +0000 |
4 |
Commit: Mike Frysinger <vapier <AT> gentoo <DOT> org> |
5 |
CommitDate: Mon Sep 28 20:01:33 2015 +0000 |
6 |
URL: https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=4377a68d |
7 |
|
8 |
libsandbox: do not unnecessarily dereference symlinks |
9 |
|
10 |
When the target uses a func that operates on a symlink, we should not |
11 |
dereference that symlink when trying to validate the call. It's both |
12 |
a waste of time and it subtly breaks code that checks atime updates. |
13 |
The act of reading symlinks is enough to cause their atime to change. |
14 |
|
15 |
URL: https://bugs.gentoo.org/415475 |
16 |
Reported-by: Marien Zwart <marienz <AT> gentoo.org> |
17 |
Signed-off-by: Mike Frysinger <vapier <AT> gentoo.org> |
18 |
|
19 |
libsandbox/libsandbox.c | 15 ++++++++++++--- |
20 |
tests/utimensat-4.sh | 30 ++++++++++++++++++++++++++++++ |
21 |
tests/utimensat.at | 1 + |
22 |
3 files changed, 43 insertions(+), 3 deletions(-) |
23 |
|
24 |
diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c |
25 |
index 1d9fa04..2bcff95 100644 |
26 |
--- a/libsandbox/libsandbox.c |
27 |
+++ b/libsandbox/libsandbox.c |
28 |
@@ -909,7 +909,14 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func, |
29 |
bool access, debug, verbose, set; |
30 |
|
31 |
absolute_path = resolve_path(file, 0); |
32 |
- resolved_path = resolve_path(file, 1); |
33 |
+ /* Do not bother dereferencing symlinks when we are using a function that |
34 |
+ * itself does not dereference. This speeds things up and avoids updating |
35 |
+ * the atime implicitly. #415475 |
36 |
+ */ |
37 |
+ if (symlink_func(sb_nr, flags, absolute_path)) |
38 |
+ resolved_path = absolute_path; |
39 |
+ else |
40 |
+ resolved_path = resolve_path(file, 1); |
41 |
if (!absolute_path || !resolved_path) |
42 |
goto error; |
43 |
sb_debug_dyn("absolute_path: %s\n", absolute_path); |
44 |
@@ -955,7 +962,8 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func, |
45 |
} |
46 |
|
47 |
free(absolute_path); |
48 |
- free(resolved_path); |
49 |
+ if (absolute_path != resolved_path) |
50 |
+ free(resolved_path); |
51 |
|
52 |
errno = old_errno; |
53 |
|
54 |
@@ -967,7 +975,8 @@ static int check_syscall(sbcontext_t *sbcontext, int sb_nr, const char *func, |
55 |
*/ |
56 |
if (errno_is_too_long()) { |
57 |
free(absolute_path); |
58 |
- free(resolved_path); |
59 |
+ if (absolute_path != resolved_path) |
60 |
+ free(resolved_path); |
61 |
return 2; |
62 |
} |
63 |
|
64 |
|
65 |
diff --git a/tests/utimensat-4.sh b/tests/utimensat-4.sh |
66 |
new file mode 100755 |
67 |
index 0000000..731c7d1 |
68 |
--- /dev/null |
69 |
+++ b/tests/utimensat-4.sh |
70 |
@@ -0,0 +1,30 @@ |
71 |
+#!/bin/sh |
72 |
+# make sure we don't accidentally trip atime updates on files |
73 |
+# through symlinks #415475 |
74 |
+[ "${at_xfail}" = "yes" ] && exit 77 # see script-0 |
75 |
+ |
76 |
+# We assume $PWD supports atimes, and the granularity is more than 1 second. |
77 |
+# If it doesn't, this test will still pass, but not really because the code |
78 |
+# was proven to be correct. |
79 |
+ |
80 |
+# XXX: Maybe we need to add our own stat shim to avoid portability issues ? |
81 |
+get_atime() { |
82 |
+ # This shows the full atime field (secs, msecs, nsecs). |
83 |
+ stat -c %x "$1" |
84 |
+} |
85 |
+ |
86 |
+# Create a symlink. |
87 |
+sym="sym" |
88 |
+ln -s atime "${sym}" |
89 |
+ |
90 |
+# Get the state before we test it. |
91 |
+before=$(get_atime "${sym}") |
92 |
+ |
93 |
+# A quick sleep of a few msecs. |
94 |
+sleep 0.1 |
95 |
+ |
96 |
+# See if the atime changes -- it should not. |
97 |
+utimensat-0 -1,EINVAL AT_FDCWD "${sym}" -1,-1 AT_SYMLINK_NOFOLLOW || exit 1 |
98 |
+after=$(get_atime "${sym}") |
99 |
+ |
100 |
+[ "${after}" = "${before}" ] |
101 |
|
102 |
diff --git a/tests/utimensat.at b/tests/utimensat.at |
103 |
index eec4638..1909650 100644 |
104 |
--- a/tests/utimensat.at |
105 |
+++ b/tests/utimensat.at |
106 |
@@ -1,3 +1,4 @@ |
107 |
SB_CHECK(1) |
108 |
SB_CHECK(2) |
109 |
SB_CHECK(3) |
110 |
+SB_CHECK(4) |