Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proc/linux: Don't open mmapped files before checking stat results #2002

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

hillu
Copy link
Contributor

@hillu hillu commented Nov 3, 2023

Instead of checking the filesystem containing the mapped file against specific magic device numbers, we use stat(2) before actually opening the file. (The open call can block on certain device nodes as reported in #1929.)

To overcome the possible TOCTOU issue between stat() and open(), we double-check device, inode after we have opened the file.

This also reverts commit 50eeef4.

The original problem could also have been fixed by passing O_PATH to open(2). However, there seems to be no way to atomically upgrade those path-only fds to real file descriptors that we can mmap or read from, so this would not helped in overcoming the TOCTOU issue.

Instead of checking the filesystem containing the mapped file against
specific magic device numbers, we use stat(2) before actually opening
the file. (The open call can block on certain device nodes as reported
in VirusTotal#1929.)

To overcome the possible TOCTOU issue between stat() and open(), we
double-check device, inode after we have opened the file.

This also reverts commit 50eeef4.

The original problem could also have been fixed by passing O_PATH to
open(2). However, there seems to be no way to atomically upgrade those
path-only fds to real file descriptors that we can mmap or read from,
so this would not helped in overcoming the TOCTOU issue.
@plusvic plusvic merged commit 2967e86 into VirusTotal:master Nov 7, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants