Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 23
Harden hypervisor process liveness checks#363
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
Open
yummybomb
wants to merge
48
commits into
hypeship/generalize-vgpu-device
from
hypeship/hypervisor-liveness
Uh oh!
There was an error while loading. Please reload this page.
Open
Changes from all commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
ea498ad
Unify hypervisor liveness checks on ProcessExists
yummybomb 86710af
Wait for non-child hypervisor exit before finishing kill
yummybomb 017ce62
Verify socket ownership before treating a hypervisor PID as live
yummybomb 4ba2222
Fail closed on hypervisor liveness checks
yummybomb 92c5966
Fail closed on duplicate socket paths
yummybomb 02c1d23
Resolve socket owner from listening entries only
yummybomb 7c90df3
Verify socket ownership before force-killing a hypervisor PID
yummybomb d283052
Skip hypervisor kill when socket ownership is unconfirmed
yummybomb d051a9b
Fail delete when hypervisor ownership is unconfirmed
yummybomb e960315
Verify hypervisor ownership before killing
yummybomb afc4205
Fail closed on unconfirmed socket match with no stored PID
yummybomb 8bcebb2
Treat unsignalable hypervisor processes as alive
yummybomb ed3ffd5
Document fail-closed hypervisor errors
yummybomb 6d765be
Handle process exit races during socket scans
yummybomb 6bb9b91
Confirm hypervisor identity before kill
yummybomb 059628d
Handle hypervisor identity edge cases
yummybomb 2b8e651
Disambiguate inherited hypervisor sockets
yummybomb f8021dc
Add non-Linux process owner resolver
yummybomb 8cfc830
Scope hypervisor identity to host boot
yummybomb 4f6c53c
Verify graceful shutdown process ownership
yummybomb 64c1a75
Mint hypervisor identity tokens only for confirmed PIDs
yummybomb c0afcbc
Treat a hypervisor identity from a previous boot as dead
yummybomb 0264733
Treat a socket with no owning process as proof the hypervisor is gone
yummybomb b668187
Confirm the expected owner's socket fd before scanning all of /proc
yummybomb 85496d5
Backfill hypervisor process identity at startup
yummybomb a0d4c49
Memoize the host boot ID
yummybomb 70b3b93
Skip unreadable fds in the candidate socket ownership check
yummybomb 79d7f2d
Record a bare PID when the fallback hypervisor PID is dead
yummybomb 99eed9f
Resolve hypervisor ownership before shutdown kill
yummybomb 7dace03
Handle dead owners in shutdown and socket classification
yummybomb ed61a3b
Keep the fail-closed resolver off the hydration hot path
yummybomb f374510
Extract hypervisor process identity logic into process_identity.go
yummybomb d7383f8
Group hypervisor process identity fields into a struct
yummybomb 52f0ab2
Collapse the three SIGKILL-and-wait paths into one helper
yummybomb bbddd62
Log a summary line after hypervisor identity backfill
yummybomb c2cb5a6
Reduce hypervisor SIGKILL wait from 30s to 2s
yummybomb e56150f
Defer stuck delete teardown to a background finalizer
yummybomb 7739c3c
Revert "Defer stuck delete teardown to a background finalizer"
yummybomb 43fe762
Drop unused identity checks and redundant kill-wait constant
yummybomb 2ae7af4
Consolidate redundant identity tests
yummybomb d24922c
Merge forceKillHypervisorProcess into killHypervisor
yummybomb 037fed6
Abort standby when the hypervisor cannot be confirmed dead
yummybomb 83fdb81
Remove the hypervisor socket only after confirmed exit
yummybomb 09296ec
Remove the command-line fallback from socket owner resolution
yummybomb 370aa1d
Skip the force-kill fallback after a confirmed hypervisor shutdown
yummybomb 193c5aa
Remove the startup hypervisor identity backfill
yummybomb 07d7fac
Reap zombie child VMMs and scan /proc in the churn test
yummybomb f42204d
Retry cleanup deletes until the hypervisor teardown converges
yummybomb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| package hypervisor | ||
| import "errors" | ||
| var ErrNoOwningProcess = errors.New("no owning process found") |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4,70 +4,77 @@ package hypervisor | ||
| import ( | ||
| "bufio" | ||
| "errors" | ||
| "fmt" | ||
| "io/fs" | ||
| "os" | ||
| "path/filepath" | ||
| "slices" | ||
| "strconv" | ||
| "strings" | ||
| "syscall" | ||
| ) | ||
| var procDir = "/proc" | ||
| // soAcceptcon marks a listening socket in /proc/net/unix (__SO_ACCEPTCON). | ||
| const soAcceptcon = 0x10000 | ||
| // ResolveProcessPID finds the process currently holding the listening Unix | ||
| // socket for the given hypervisor control path. | ||
| func ResolveProcessPID(socketPath string) (int, error) { | ||
| // socket for the given hypervisor control path, via the socket inode in | ||
| // /proc/net/unix and each process's fd table. The fd scan requires the | ||
| // caller to hold CAP_SYS_PTRACE (or run as root) so no live owner is missed; | ||
| // an ErrNoOwningProcess result is proof the listener is gone. | ||
| func ResolveProcessPID(socketPath string) (pid int, err error) { | ||
| return resolveProcessPID(socketPath, 0) | ||
| } | ||
| // ResolveProcessPIDForOwner resolves a socket while preferring an expected | ||
| // owner when the socket descriptor is temporarily shared with a child process. | ||
| func ResolveProcessPIDForOwner(socketPath string, ownerPID int) (pid int, err error) { | ||
| return resolveProcessPID(socketPath, ownerPID) | ||
| } | ||
| func resolveProcessPID(socketPath string, ownerPID int) (pid int, err error) { | ||
| socketRef, err := socketRefForPath(socketPath) | ||
| if err == nil { | ||
| if pid, refErr := pidBySocketRef(socketRef); refErr == nil { | ||
| return pid, nil | ||
| } | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| if pid, cmdErr := pidByCmdline(socketPath); cmdErr == nil { | ||
| return pid, nil | ||
| // Confirm the expected owner first so a live stored PID does not | ||
| // require scanning every process fd. | ||
| if ownerPID > 0 && processHoldsSocketRef(ownerPID, socketRef) { | ||
| return ownerPID, nil | ||
| } | ||
| return 0, fmt.Errorf("resolve process pid for socket %s: no owning process found", socketPath) | ||
| return pidBySocketRef(socketRef, ownerPID) | ||
| } | ||
| func pidBySocketRef(socketRef string) (int, error) { | ||
| procEntries, err := os.ReadDir("/proc") | ||
| func processHoldsSocketRef(pid int, socketRef string) bool { | ||
| fdEntries, err := os.ReadDir(filepath.Join(procDir, strconv.Itoa(pid), "fd")) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("read /proc: %w", err) | ||
| return false | ||
| } | ||
| for _, entry := range procEntries { | ||
| if !entry.IsDir() { | ||
| continue | ||
| } | ||
| pid, err := strconv.Atoi(entry.Name()) | ||
| for _, fdEntry := range fdEntries { | ||
| target, err := os.Readlink(filepath.Join(procDir, strconv.Itoa(pid), "fd", fdEntry.Name())) | ||
| if err != nil { | ||
| // Skip fds that cannot be read, like the full scan does: an fd | ||
| // vanishing mid-scan must not hide a listener held by a later fd. | ||
| continue | ||
| } | ||
| fdEntries, err := os.ReadDir(filepath.Join("/proc", entry.Name(), "fd")) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| for _, fdEntry := range fdEntries { | ||
| target, err := os.Readlink(filepath.Join("/proc", entry.Name(), "fd", fdEntry.Name())) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| if strings.TrimSpace(target) == socketRef { | ||
| return pid, nil | ||
| } | ||
| if strings.TrimSpace(target) == socketRef { | ||
| return true | ||
| } | ||
| } | ||
| return 0, fmt.Errorf("resolve process pid for %s: no owning process found", socketRef) | ||
| return false | ||
| } | ||
| func pidByCmdline(socketPath string) (int, error) { | ||
| procEntries, err := os.ReadDir("/proc") | ||
| func pidBySocketRef(socketRef string, ownerPID int) (int, error) { | ||
| procEntries, err := os.ReadDir(procDir) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("read /proc: %w", err) | ||
| } | ||
| var owners []int | ||
| var scanErr error | ||
| for _, entry := range procEntries { | ||
| if !entry.IsDir() { | ||
| continue | ||
| @@ -78,28 +85,57 @@ func pidByCmdline(socketPath string) (int, error) { | ||
| continue | ||
| } | ||
| cmdline, err := os.ReadFile(filepath.Join("/proc", entry.Name(), "cmdline")) | ||
| if err != nil || len(cmdline) == 0 { | ||
| fdEntries, err := os.ReadDir(filepath.Join(procDir, entry.Name(), "fd")) | ||
| if err != nil { | ||
| if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ESRCH) { | ||
| continue | ||
| } | ||
| scanErr = err | ||
| continue | ||
| } | ||
| for _, arg := range strings.Split(string(cmdline), "\x00") { | ||
| if arg == socketPath { | ||
| return pid, nil | ||
| for _, fdEntry := range fdEntries { | ||
| target, err := os.Readlink(filepath.Join(procDir, entry.Name(), "fd", fdEntry.Name())) | ||
| if err != nil { | ||
| if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ESRCH) { | ||
| continue | ||
| } | ||
| scanErr = err | ||
| continue | ||
| } | ||
| if strings.TrimSpace(target) == socketRef { | ||
| owners = append(owners, pid) | ||
| break | ||
| } | ||
| } | ||
| } | ||
| return 0, fmt.Errorf("resolve process pid for socket %s: no matching command line found", socketPath) | ||
| // The scan observed ownerPID holding the listener fd — the same evidence | ||
| // the fast path uses — so a child transiently sharing the inherited fd | ||
| // must not turn a proven owner into an error. | ||
| if ownerPID > 0 && slices.Contains(owners, ownerPID) { | ||
| return ownerPID, nil | ||
| } | ||
| if len(owners) == 1 { | ||
| return owners[0], nil | ||
| } | ||
| if len(owners) > 1 { | ||
| return 0, fmt.Errorf("resolve process pid for %s: multiple owning processes found: %v", socketRef, owners) | ||
| } | ||
| if scanErr != nil { | ||
| return 0, fmt.Errorf("resolve process pid for %s: inspect process fds: %w", socketRef, scanErr) | ||
| } | ||
| return 0, fmt.Errorf("resolve process pid for %s: %w", socketRef, ErrNoOwningProcess) | ||
| } | ||
| func socketRefForPath(socketPath string) (string, error) { | ||
| file, err := os.Open("/proc/net/unix") | ||
| file, err := os.Open(filepath.Join(procDir, "net", "unix")) | ||
| if err != nil { | ||
| return "", fmt.Errorf("open /proc/net/unix: %w", err) | ||
| } | ||
| defer file.Close() | ||
| scanner := bufio.NewScanner(file) | ||
| var socketRef string | ||
| for scanner.Scan() { | ||
| fields := strings.Fields(scanner.Text()) | ||
| if len(fields) < 7 { | ||
| @@ -112,14 +148,26 @@ func socketRefForPath(socketPath string) (string, error) { | ||
| if path != socketPath { | ||
| continue | ||
| } | ||
| // Accepted server-side sockets list the bound path too; only the | ||
| // listener identifies the owning process. | ||
| flags, parseErr := strconv.ParseUint(fields[3], 16, 32) | ||
| if parseErr != nil || flags&soAcceptcon == 0 { | ||
| continue | ||
| } | ||
| inode := fields[6] | ||
| if inode == "" { | ||
| break | ||
| } | ||
| return fmt.Sprintf("socket:[%s]", inode), nil | ||
| if socketRef != "" { | ||
| return "", fmt.Errorf("resolve process pid for socket %s: multiple socket inodes found", socketPath) | ||
| } | ||
| socketRef = fmt.Sprintf("socket:[%s]", inode) | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| if err := scanner.Err(); err != nil { | ||
| return "", fmt.Errorf("scan /proc/net/unix: %w", err) | ||
| } | ||
| return "", fmt.Errorf("resolve process pid for socket %s: socket inode not found", socketPath) | ||
| if socketRef != "" { | ||
| return socketRef, nil | ||
| } | ||
| return "", fmt.Errorf("resolve process pid for socket %s: socket inode not found: %w", socketPath, ErrNoOwningProcess) | ||
| } | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.