pkg/ls:FromOSFileInfo: fix a panic if fi.Sys() returns the wrong type #2723
Conversation
This file contains 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
rminnich
force-pushed
the
fix-pkg-ls-panic
branch
from
86da062
to
edc4b46
Compare
rminnich
requested review from
hugelgupf,
abarisani,
10000TB,
andrewsun2898,
ChriMarMe,
MDr164 and
a team
FromOSFileInfo had a blind cast of a FileInfo.Sys() to *syscall.Stat_t. This could fail with a badly behaving file system, e.g. a 9p server that would fail a Stat in ways that the go runtime was not prepared for. Ask me how I know :-) The fact that we have not seen this in 11 years of use testifies to its rarity, but ... it can happen. In FromOSFileInfo, pick default values for UID, GID, and rdev Check if fi.Sys() is a *syscall.Stat_t and, if so, use Uid, Gid, and Rdev from it. Add a type to the linux test that implements os.FileInfo (yes, fs.FileInfo now ...) that can return a Sys() of the wrong type. I verified that this test catches the failing case. A bit of cleanup: Rename the test file ls_linux_test.go to fileinfo_linux_test.go, to match everything else in this directory. Remove a spurious fmt.Printf from the fileinfo_linux_test.go that added trash to the test output. Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
rminnich
force-pushed
the
fix-pkg-ls-panic
branch
from
520729f
to
cc237c1
Compare
This came was introduced as a result of trying to match how "normal" ls works. Sorry about that. Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2723 +/- ##
==========================================
- Coverage 75.41% 75.39% -0.03%
==========================================
Files 414 414
Lines 42229 42231 +2
==========================================
- Hits 31847 31838 -9
- Misses 10382 10393 +11
☔ View full report in Codecov by Sentry. |
brho
approved these changes
Jul 22, 2023
binjip978
approved these changes
Jul 22, 2023
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.
FromOSFileInfo had a blind cast of a FileInfo.Sys() to *syscall.Stat_t.
This could fail with a badly behaving file system, e.g. a 9p server that would fail a Stat in ways that the go runtime was not prepared for. Ask me how I know :-) The fact that we have not seen this in 11 years of use testifies to its rarity, but ... it can happen.
In FromOSFileInfo, pick default values for UID, GID, and rdev
Check if fi.Sys() is a *syscall.Stat_t and, if so, use Uid, Gid, and Rdev from it.
Add a type to the linux test that implements os.FileInfo (yes, fs.FileInfo now ...) that can return a Sys() of the wrong type. I verified that this test
catches the failing case.
A bit of cleanup:
Rename the test file ls_linux_test.go to fileinfo_linux_test.go, to match everything else in this directory.
Remove a spurious fmt.Printf from the fileinfo_linux_test.go that added trash to the test output.