Do not replace /dev/null with a regular file in WriteFile - #31038
Open
zakinko wants to merge 1 commit into
Open
Conversation
WriteFile unlinks its target before creating it with O_CREAT. The unlink is there so that a running executable can be replaced -- you cannot open one with O_TRUNC -- and it has been in the function since the first import, when WriteFile's job was to write executables. //src/test/cpp/util:file_test writes to /dev/null. As an ordinary user the unlink fails and the write reaches the device, so nothing happens; as root it succeeds and the node becomes a regular file. On a system with room, TestLargeFileWrite then leaves a four-gigabyte file in /dev until the next redirection truncates it. Reproduced on Debian 13 and on NetBSD 11.0/amd64. file_windows.cc already returns early for /dev/null; do the same here, and check afterwards in the POSIX test that /dev/null is still a device, so a run as root fails there instead of quietly corrupting everything after it.
|
✅ Bazel docs preview is ready! Preview URL: https://bazel-pr-31038.mintlify.app/ Updated for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WriteFile unlinks its target before creating it with O_CREAT. The unlink is
there so a running executable can be replaced, since you cannot open one with
O_TRUNC; it has been in the function since the first import, when WriteFile's
job was to write executables: the open used mode 0755 and the comment said so.
//src/test/cpp/util:file_testwrites to /dev/null, in TestWriteFile and againwith a four-gigabyte NUL-filled buffer in TestLargeFileWrite. As an ordinary
user the unlink fails and the write reaches the device, so nothing happens.
As root it succeeds and the node becomes a regular file.
On GitHub's ubuntu-latest runner, where /dev is devtmpfs, at 870abbb,
running the test as root:
Debian 13 in a container and NetBSD 11.0/amd64 do the same. On NetBSD,
polling once a second while TestLargeFileWrite runs:
/dev holds a four-gigabyte file for about a second, until the next thing that
redirects to /dev/null truncates it.
What follows the missing device is harder to find than the device itself:
2>/dev/nullbecomes a write, and the next program to read /dev/null getswhatever was left there.
file_windows.cc already returns early for /dev/null. This is the same three
lines on the POSIX side. The POSIX test then writes to /dev/null and checks
that it is still a device: as an ordinary user that cannot fail, and as root
it fails there instead of quietly corrupting everything that runs after it.
file_test passes as an ordinary user on ubuntu-latest, macos-latest and
windows-latest (the Windows build does not compile either file touched here).
The tests that fail as root do so before and after alike:
FilePosixTest.CanAccess and FileTest.TestRemoveRecursivelyPosix.