Skip to content

Commit d13e962

Browse files
committed
libutil: Implement unix::fchmodatTryNoFollow
Using fchmodat after a fstatat in deletePath has a slight TOCTOU window. We can plug it by using fchmodat (the libc wrapper with AT_SYMLINK_NOFOLLOW), but it tries fchmodat2 and falls back to the O_PATH trick while failing when procfs isn't mounted. We can do a bit better than that and also cache whether syscalls are unsupported to avoid the repeated context switching that glibc would impose. Also tests the fallback path. It's only for kernels older than 6.6 and when procfs isn't accessible that we fall back to the racy fchmodat without AT_SYMLINK_NOFOLLOW. What previously used to be: openat(AT_FDCWD, "/tmp/store-race/nix/var/nix/builds", O_RDONLY) = 11 newfstatat(11, "nix-2704212-84654554", {st_mode=S_IFDIR|000, st_size=3, ...}, AT_SYMLINK_NOFOLLOW) = 0 fchmodat(11, "nix-2704212-84654554", 040700) = 0 Is now a TOCTOU-free sequence of syscalls: openat(AT_FDCWD, "/tmp/store-race/nix/var/nix/builds", O_RDONLY) = 11 newfstatat(11, "nix-2704953-1733606057", {st_mode=S_IFDIR|000, st_size=3, ...}, AT_SYMLINK_NOFOLLOW) = 0 fchmodat2(11, "nix-2704953-1733606057", 040700, AT_SYMLINK_NOFOLLOW) = 0 Or if the fchmodat2 is not supported: openat(11, "nix-2705443-3010460784", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH) = 12 fstat(12, {st_mode=S_IFDIR|000, st_size=3, ...}) = 0 chmod("/proc/self/fd/12", 040700) = 0 openat(11, "nix-2705443-3010460784", O_RDONLY|O_NOFOLLOW|O_DIRECTORY) = 12 This prevents a potentially arbitrary chmod that follows symlinks, though the race window is very small. Also in the case that fchmodat2 isn't supported we could instead open the /proc/self/fd/N path instead of using openat, but that's pretty much equivalent. We only care about ensuring that the thing we chmodded wasn't a symlink since fchmodat follows symlinks and the support for AT_SYMLINK_NOFOLLOW in libc for that is pretty spotty on Linux. E.g. glibc fails if the AT_SYMLINK_NOFOLLOW is specified and procfs isn't available even on regular files. The patch also includes a test that uses a user namespace on Linux to test this exact scenario (though it's rather exotic).
1 parent 75da37f commit d13e962

File tree

5 files changed

+195
-2
lines changed

5 files changed

+195
-2
lines changed

src/libutil-tests/package.nix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ mkMesonExecutable (finalAttrs: {
3232
../../.version
3333
./.version
3434
./meson.build
35+
./unix/meson.build
3536
# ./meson.options
3637
(fileset.fileFilter (file: file.hasExt "cc") ./.)
3738
(fileset.fileFilter (file: file.hasExt "hh") ./.)

src/libutil-tests/unix/file-descriptor.cc

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,20 @@
33
#include "nix/util/file-descriptor.hh"
44
#include "nix/util/file-system.hh"
55
#include "nix/util/fs-sink.hh"
6+
#include "nix/util/processes.hh"
7+
8+
#ifdef __linux__
9+
# include "nix/util/linux-namespaces.hh"
10+
11+
# include <sys/mount.h>
12+
#endif
613

714
#include <cstring>
815

916
namespace nix {
1017

18+
using namespace nix::unix;
19+
1120
/* ----------------------------------------------------------------------------
1221
* openFileEnsureBeneathNoSymlinks
1322
* --------------------------------------------------------------------------*/
@@ -16,7 +25,6 @@ TEST(openFileEnsureBeneathNoSymlinks, works)
1625
{
1726
std::filesystem::path tmpDir = nix::createTempDir();
1827
nix::AutoDelete delTmpDir(tmpDir, /*recursive=*/true);
19-
using namespace nix::unix;
2028

2129
{
2230
RestoreSink sink(/*startFsync=*/false);
@@ -66,4 +74,106 @@ TEST(openFileEnsureBeneathNoSymlinks, works)
6674
EXPECT_TRUE(AutoCloseFD{open("a/regular", O_CREAT | O_WRONLY | O_EXCL, 0666)});
6775
}
6876

77+
/* ----------------------------------------------------------------------------
78+
* fchmodatTryNoFollow
79+
* --------------------------------------------------------------------------*/
80+
81+
TEST(fchmodatTryNoFollow, works)
82+
{
83+
std::filesystem::path tmpDir = nix::createTempDir();
84+
nix::AutoDelete delTmpDir(tmpDir, /*recursive=*/true);
85+
86+
{
87+
RestoreSink sink(/*startFsync=*/false);
88+
sink.dstPath = tmpDir;
89+
sink.dirFd = openDirectory(tmpDir);
90+
sink.createRegularFile(CanonPath("file"), [](CreateRegularFileSink & crf) {});
91+
sink.createDirectory(CanonPath("dir"));
92+
sink.createSymlink(CanonPath("filelink"), "file");
93+
sink.createSymlink(CanonPath("dirlink"), "dir");
94+
}
95+
96+
ASSERT_EQ(chmod((tmpDir / "file").c_str(), 0644), 0);
97+
ASSERT_EQ(chmod((tmpDir / "dir").c_str(), 0755), 0);
98+
99+
AutoCloseFD dirFd = openDirectory(tmpDir);
100+
ASSERT_TRUE(dirFd);
101+
102+
struct ::stat st;
103+
104+
/* Check that symlinks are not followed and targets are not changed. */
105+
106+
fchmodatTryNoFollow(dirFd.get(), CanonPath("filelink"), 0777);
107+
ASSERT_EQ(stat((tmpDir / "file").c_str(), &st), 0);
108+
EXPECT_EQ(st.st_mode & 0777, 0644);
109+
110+
fchmodatTryNoFollow(dirFd.get(), CanonPath("dirlink"), 0777);
111+
ASSERT_EQ(stat((tmpDir / "dir").c_str(), &st), 0);
112+
EXPECT_EQ(st.st_mode & 0777, 0755);
113+
114+
/* Check fchmodatTryNoFollow works on regular files and directories. */
115+
116+
EXPECT_EQ(fchmodatTryNoFollow(dirFd.get(), CanonPath("file"), 0600), 0);
117+
ASSERT_EQ(stat((tmpDir / "file").c_str(), &st), 0);
118+
EXPECT_EQ(st.st_mode & 0777, 0600);
119+
120+
EXPECT_EQ(fchmodatTryNoFollow(dirFd.get(), CanonPath("dir"), 0700), 0);
121+
ASSERT_EQ(stat((tmpDir / "dir").c_str(), &st), 0);
122+
EXPECT_EQ(st.st_mode & 0777, 0700);
123+
}
124+
125+
#ifdef __linux__
126+
127+
TEST(fchmodatTryNoFollow, fallbackWithoutProc)
128+
{
129+
if (!userNamespacesSupported())
130+
GTEST_SKIP() << "User namespaces not supported";
131+
132+
std::filesystem::path tmpDir = nix::createTempDir();
133+
nix::AutoDelete delTmpDir(tmpDir, /*recursive=*/true);
134+
135+
{
136+
RestoreSink sink(/*startFsync=*/false);
137+
sink.dstPath = tmpDir;
138+
sink.dirFd = openDirectory(tmpDir);
139+
sink.createRegularFile(CanonPath("file"), [](CreateRegularFileSink & crf) {});
140+
sink.createSymlink(CanonPath("link"), "file");
141+
}
142+
143+
ASSERT_EQ(chmod((tmpDir / "file").c_str(), 0644), 0);
144+
145+
Pid pid = startProcess(
146+
[&] {
147+
if (unshare(CLONE_NEWNS) == -1)
148+
_exit(1);
149+
150+
if (mount(0, "/", 0, MS_PRIVATE | MS_REC, 0) == -1)
151+
_exit(1);
152+
153+
if (mount("tmpfs", "/proc", "tmpfs", 0, 0) == -1)
154+
_exit(1);
155+
156+
AutoCloseFD dirFd = openDirectory(tmpDir);
157+
if (!dirFd)
158+
exit(1);
159+
160+
if (fchmodatTryNoFollow(dirFd.get(), CanonPath("file"), 0600) != 0)
161+
_exit(1);
162+
163+
if (fchmodatTryNoFollow(dirFd.get(), CanonPath("link"), 0777) != -1)
164+
_exit(1);
165+
166+
_exit(0);
167+
},
168+
{.cloneFlags = CLONE_NEWUSER});
169+
170+
int status = pid.wait();
171+
ASSERT_TRUE(statusOk(status));
172+
173+
struct ::stat st;
174+
ASSERT_EQ(stat((tmpDir / "file").c_str(), &st), 0);
175+
EXPECT_EQ(st.st_mode & 0777, 0600);
176+
}
177+
#endif
178+
69179
} // namespace nix

src/libutil/include/nix/util/file-descriptor.hh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,16 @@ namespace unix {
257257
*/
258258
Descriptor openFileEnsureBeneathNoSymlinks(Descriptor dirFd, const CanonPath & path, int flags, mode_t mode = 0);
259259

260+
/**
261+
* Try to change the mode of file named by \ref path relative to the parent directory denoted by \ref dirFd.
262+
*
263+
* @note When on linux without fchmodat2 support and without procfs mounted falls back to fchmodat without
264+
* AT_SYMLINK_NOFOLLOW, since it's the best we can do without failing.
265+
*
266+
* @pre path.isRoot() is false
267+
*/
268+
int fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t mode);
269+
260270
} // namespace unix
261271
#endif
262272

src/libutil/unix/file-descriptor.cc

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@
1717
# define HAVE_OPENAT2 0
1818
#endif
1919

20+
#if defined(__linux__) && defined(__NR_fchmodat2)
21+
# define HAVE_FCHMODAT2 1
22+
#else
23+
# define HAVE_FCHMODAT2 0
24+
#endif
25+
2026
#include "util-config-private.hh"
2127
#include "util-unix-config-private.hh"
2228

@@ -265,6 +271,72 @@ std::optional<Descriptor> openat2(Descriptor dirFd, const char * path, uint64_t
265271

266272
#endif
267273

274+
int unix::fchmodatTryNoFollow(Descriptor dirFd, const CanonPath & path, mode_t mode)
275+
{
276+
assert(!path.isRoot());
277+
278+
#if HAVE_FCHMODAT2
279+
/* Cache whether fchmodat2 is not supported. */
280+
static std::atomic_flag fchmodat2Unsupported{};
281+
if (!fchmodat2Unsupported.test()) {
282+
/* Try with fchmodat2 first. */
283+
auto res = ::syscall(__NR_fchmodat2, dirFd, path.rel_c_str(), mode, AT_SYMLINK_NOFOLLOW);
284+
/* Cache that the syscall is not supported. */
285+
if (res < 0 && errno == ENOSYS) {
286+
fchmodat2Unsupported.test_and_set();
287+
} else {
288+
return res;
289+
}
290+
}
291+
#endif
292+
293+
#ifdef __linux__
294+
AutoCloseFD pathFd = ::openat(dirFd, path.rel_c_str(), O_PATH | O_NOFOLLOW | O_CLOEXEC);
295+
if (!pathFd)
296+
throw SysError(
297+
"opening '%s' relative to parent directory to get an O_PATH file descriptor (fchmodat2 is unsupported)",
298+
path.rel());
299+
300+
struct ::stat st;
301+
/* Possible since https://github.com/torvalds/linux/commit/55815f70147dcfa3ead5738fd56d3574e2e3c1c2 (3.6) */
302+
if (::fstat(pathFd.get(), &st) == -1)
303+
throw SysError("statting '%s' relative to parent directory via O_PATH file descriptor", path.rel());
304+
305+
if (S_ISLNK(st.st_mode)) {
306+
errno = EOPNOTSUPP;
307+
return -1;
308+
}
309+
310+
static std::atomic_flag dontHaveProc{};
311+
if (!dontHaveProc.test()) {
312+
static const CanonPath selfProcFd = CanonPath("/proc/self/fd");
313+
314+
auto selfProcFdPath = selfProcFd / std::to_string(pathFd.get());
315+
if (int res = ::chmod(selfProcFdPath.c_str(), mode); res == -1) {
316+
if (errno == ENOENT)
317+
dontHaveProc.test_and_set();
318+
else
319+
throw SysError("chmod '%s' ('%s' relative to parent directory)", selfProcFdPath, path);
320+
} else
321+
return res;
322+
}
323+
324+
static std::atomic<bool> warned = false;
325+
warnOnce(warned, "kernel doesn't support fchmodat2 and procfs isn't mounted, falling back to fchmodat");
326+
#endif
327+
328+
return ::fchmodat(
329+
dirFd,
330+
path.rel_c_str(),
331+
mode,
332+
#if defined(__APPLE__) || defined(__FreeBSD__)
333+
AT_SYMLINK_NOFOLLOW
334+
#else
335+
0
336+
#endif
337+
);
338+
}
339+
268340
static Descriptor
269341
openFileEnsureBeneathNoSymlinksIterative(Descriptor dirFd, const CanonPath & path, int flags, mode_t mode)
270342
{

src/libutil/unix/file-system.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ static void _deletePath(
140140
/* Make the directory accessible. */
141141
const auto PERM_MASK = S_IRUSR | S_IWUSR | S_IXUSR;
142142
if ((st.st_mode & PERM_MASK) != PERM_MASK) {
143-
if (fchmodat(parentfd, name.c_str(), st.st_mode | PERM_MASK, 0) == -1)
143+
if (unix::fchmodatTryNoFollow(parentfd, CanonPath(name), st.st_mode | PERM_MASK) == -1)
144144
throw SysError("chmod %1%", path);
145145
}
146146

0 commit comments

Comments
 (0)