Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/libexpr/eval-profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,17 @@ class SampleStack : public EvalProfiler
FrameInfo getPrimOpFrameInfo(const PrimOp & primOp, std::span<Value *> args, PosIdx pos);

public:
SampleStack(EvalState & state, std::filesystem::path profileFile, std::chrono::nanoseconds period)
SampleStack(EvalState & state, const std::filesystem::path & profileFile, std::chrono::nanoseconds period)
: state(state)
, sampleInterval(period)
, profileFd([&]() {
AutoCloseFD fd = toDescriptor(open(profileFile.string().c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0660));
AutoCloseFD fd = openNewFileForWrite(
profileFile,
0660,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why I did it this way. Generally we override the umask to 0022 everywhere

{
.truncateExisting = true,
.followSymlinksOnTruncate = true, /* FIXME: Probably shouldn't follow symlinks. */
});
if (!fd)
throw SysError("opening file %s", PathFmt(profileFile));
return fd;
Expand Down
15 changes: 7 additions & 8 deletions src/libstore/build/derivation-building-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1035,14 +1035,13 @@ LogFile::LogFile(Store & store, const StorePath & drvPath, const LogFileSettings

Path logFileName = fmt("%s/%s%s", dir, baseName.substr(2), logSettings.compressLog ? ".bz2" : "");

fd = toDescriptor(open(
logFileName.c_str(),
O_CREAT | O_WRONLY | O_TRUNC
#ifndef _WIN32
| O_CLOEXEC
#endif
,
0666));
fd = openNewFileForWrite(
logFileName,
0666,
{
.truncateExisting = true,
.followSymlinksOnTruncate = true, /* FIXME: Probably shouldn't follow symlinks. */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure we never expect this to be able to become a symlink, unless the user is doing something very naughty

});
if (!fd)
throw SysError("creating log file '%1%'", logFileName);

Expand Down
23 changes: 23 additions & 0 deletions src/libutil/include/nix/util/file-system.hh
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,29 @@ Descriptor openDirectory(const std::filesystem::path & path);
*/
Descriptor openFileReadonly(const std::filesystem::path & path);

struct OpenNewFileForWriteParams
{
/**
* Whether to truncate an existing file.
*/
bool truncateExisting:1 = false;
/**
* Whether to follow symlinks if @ref truncateExisting is true.
*/
bool followSymlinksOnTruncate:1 = false;
};

/**
* Open a `Descriptor` for write access or create it if it doesn't exist or truncate existing depending on @ref
* truncateExisting.
*
* @param mode POSIX permission bits. Ignored on Windows.
* @throws Nothing.
*
* @todo Reparse points on Windows.
*/
Descriptor openNewFileForWrite(const std::filesystem::path & path, mode_t mode, OpenNewFileForWriteParams params);

/**
* Read the contents of a file into a string.
*/
Expand Down
13 changes: 13 additions & 0 deletions src/libutil/unix/file-system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ Descriptor openFileReadonly(const std::filesystem::path & path)
return open(path.c_str(), O_RDONLY | O_CLOEXEC);
}

Descriptor openNewFileForWrite(const std::filesystem::path & path, mode_t mode, OpenNewFileForWriteParams params)
{
auto flags = O_WRONLY | O_CREAT | O_CLOEXEC;
if (params.truncateExisting) {
flags |= O_TRUNC;
if (!params.followSymlinksOnTruncate)
flags |= O_NOFOLLOW;
} else {
flags |= O_EXCL; /* O_CREAT | O_EXCL already ensures that symlinks are not followed. */
}
return open(path.c_str(), flags, mode);
}

std::filesystem::path defaultTempDir()
{
return getEnvNonEmpty("TMPDIR").value_or("/tmp");
Expand Down
13 changes: 13 additions & 0 deletions src/libutil/windows/file-system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,19 @@ Descriptor openFileReadonly(const std::filesystem::path & path)
/*hTemplateFile=*/nullptr);
}

Descriptor
openNewFileForWrite(const std::filesystem::path & path, [[maybe_unused]] mode_t mode, OpenNewFileForWriteParams params)
{
return CreateFileW(
path.c_str(),
GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_DELETE,
/*lpSecurityAttributes=*/nullptr,
params.truncateExisting ? CREATE_ALWAYS : CREATE_NEW, /* TODO: Reparse points. */
FILE_ATTRIBUTE_NORMAL,
/*hTemplateFile=*/nullptr);
}

std::filesystem::path defaultTempDir()
{
wchar_t buf[MAX_PATH + 1];
Expand Down
8 changes: 7 additions & 1 deletion src/nix/nix-channel/nix-channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ static void readChannels()
// Writes the list of channels.
static void writeChannels()
{
auto channelsFD = AutoCloseFD{open(channelsList.c_str(), O_WRONLY | O_CLOEXEC | O_CREAT | O_TRUNC, 0644)};
AutoCloseFD channelsFD = openNewFileForWrite(
channelsList,
0644,
{
.truncateExisting = true,
.followSymlinksOnTruncate = true, /* Back-compat. */
});
if (!channelsFD)
throw SysError("opening '%1%' for writing", channelsList.string());
for (const auto & channel : channels)
Expand Down
6 changes: 3 additions & 3 deletions src/nix/prefetch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ std::tuple<StorePath, Hash> prefetchFile(
if (executable)
mode = 0700;

AutoCloseFD fd = toDescriptor(open(tmpFile.string().c_str(), O_WRONLY | O_CREAT | O_EXCL, mode));
AutoCloseFD fd = openNewFileForWrite(tmpFile, mode, {.truncateExisting = false});
if (!fd)
throw SysError("creating temporary file %s", PathFmt(tmpFile));

Expand All @@ -126,9 +126,9 @@ std::tuple<StorePath, Hash> prefetchFile(
/* Optionally unpack the file. */
if (unpack) {
Activity act(*logger, lvlChatty, actUnknown, fmt("unpacking '%s'", url.to_string()));
auto unpacked = (tmpDir.path() / "unpacked").string();
auto unpacked = tmpDir.path() / "unpacked";
createDirs(unpacked);
unpackTarfile(tmpFile.string(), unpacked);
unpackTarfile(tmpFile, unpacked);

auto entries = DirectoryIterator{unpacked};
/* If the archive unpacks to a single file/directory, then use
Expand Down
Loading