Repository navigation
Verify a device node carries the number it was created with - #405
Merged
Merged
Conversation
CreateDevice passed on whatever error mknod returned and never checked the result, so a filesystem that accepts a device number without recording it produced a node pointing at the wrong device, silently and with a successful exit. Two platforms are known to do exactly that. DragonFly's mknod stores NODEV. NetBSD's did the same through mknodat until #403 switched it to mknod. Neither reported anything: what gave NetBSD away was a test that happens to run as root in a VM job, and nothing would have given away a filesystem that behaved the same way under Linux or macOS, because that test never runs there - GitHub's hosted runners aren't root. So read the node back and compare. Whether a device number survives depends on the kernel and on the filesystem underneath rather than on the operating system, so this isn't something a build tag can express. An extraction that stops and says so beats a device node pointing somewhere else. On DragonFly untar now refuses device entries instead of writing wrong ones, which is the intended trade and is noted in the platform table. Verified as root on Linux, where the round-trip test had never actually run before: it passes, and a deliberate mismatch is caught with the expected message.
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.
CreateDevicepassed on whatever errormknodreturned and never checked the result:So a filesystem that accepts a device number without recording it produced a node pointing at the wrong device — silently, with a successful exit.
This isn't hypothetical, and it isn't one platform
Two are known to do exactly that:
mknodreports success and storesNODEV.mknodatuntil Support NetBSD, OpenBSD and DragonFly #403 switched it tomknod. Before that fix,untarthere wroterdev-0 nodes and reported success. That is fixed on master; it is listed here because it is the second independent instance of the same silent failure, not because it is still broken.Neither reports anything. NetBSD was caught only because
TestCreateDeviceRoundTripruns as root in a VM job. A filesystem behaving the same way under Linux or macOS would go unnoticed, because that test never runs there — GitHub's hosted runners are not root, so it skips:Whether a device number survives depends on the kernel and the filesystem underneath, not on the operating system, so it isn't something a build tag can express. Overlay, network and FUSE filesystems are the obvious places this could bite on Linux.
The change
Read the node back after creating it and compare major/minor, in the shared
CreateDevicerather than the per-platform files. Onelstatper device node, and device nodes are rare in archives.On DragonFly
untarnow refuses device entries instead of writing wrong ones. That's the intended trade — an extraction that stops and says so beats a node pointing somewhere else — and the platform table says so.Verification
Run as root, which no GitHub-hosted runner does:
TestCreateDeviceRoundTrippasses as root on Linux, where it is otherwise always skipped. Linux records device numbers faithfully and the check raises no false failures.TestCreateDeviceVerifiesNumber(new, portable) creates a node as 1:3 and verifies it against 9:9, standing in for a filesystem that ignores the number. It's caught with the expected message. This covers detection of a mismatch; the round-trip test alone only shows the check does not trip over a correct node.TestCreateDeviceRejectsUnrecordedNumber(new, DragonFly-only) covers the real integration case in the DragonFly VM job. Should DragonFly'smknodever start recording the number, it fails, flagging that the skip can be removed.golangci-lint, andgo vetacross linux/darwin/windows and all four BSDs.Builds on #403, now merged — that is where both instances surfaced, and its VM jobs are what give the BSD coverage this relies on. This branch is cut from master with #403 in it, so it needs nothing else first.