Gentoo Archives: gentoo-commits

From: Mike Frysinger <vapier@g.o>
To: gentoo-commits@l.g.o
Subject: [gentoo-commits] proj/sandbox:master commit in: tests/, libsandbox/
Date: Mon, 28 Sep 2015 20:17:11
Message-Id: 1443470493.4377a68df2a20cda06aadb58c179ce2e8d78f7cd.vapier@gentoo
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)