restore: pages file is parsed before the runsc_version check
- Dominant language
- Go
- Stars
- 19.3k
- Forks
- 2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 264
Description
In `runsc/boot/controller.go`, the restore path starts decoding the pages file before it
has checked that the checkpoint was written by this binary.
The version check is here:
```go
checkpointVersion := cm.restorer.metadata[VersionKey]
currentVersion := version.Version()
if checkpointVersion != currentVersion {
return fmt.Errorf("runsc version does not match across checkpoint restore, checkpoint: %v current: %v", checkpointVersion, currentVersion)
}
```
But earlier in the same function, when `pages_meta.img` is present so `HavePagesFile` is
true:
```go
if o.HavePagesFile {
// This immediately starts loading the main MemoryFile asynchronously.
cm.restorer.asyncMFLoader = kernel.NewAsyncMFLoader(pagesMetadata, pagesFile, cm.restorer.mainMF, timer.Fork("PagesFileLoader")) // transfers ownership
...
}
```
`NewAsyncMFLoader` spawns a goroutine that begins parsing the pages file straight away,
while `runsc_version` is only read and compared afterwards. So a checkpoint written by a
different build can be partially decoded before it is established that this binary should
be reading it at all.
To be clear about what this does and does not reproduce, because my first version of this
report overstated it:
**On a stock release pair, the version check works correctly.** Tested with unmodified
upstream binaries, `release-20260810.0` (checkpoint) and `release-20260817.0` (restore),
using plain `runsc checkpoint` / `runsc restore` with no containerd or Kubernetes:
$ stock-runsc-20260810 --version | head -1
runsc version release-20260810.0
$ stock-runsc-20260817 --version | head -1
runsc version release-20260817.0
$ stock-runsc-20260810 --root=/poc/root --network=none run --detach --bundle /poc/bundle ctr810
$ stock-runsc-20260810 --root=/poc/root checkpoint --image-path=/poc/cpstock ctr810
$ ls /poc/cpstock
checkpoint.img pages.img pages_meta.img
$ stock-runsc-20260817 --root=/poc/root817 --network=none \
restore --image-path=/poc/cpstock --bundle /poc/bundle ctr817
starting container: restoring container "ctr817": runsc version does not match across checkpoint restore, checkpoint: release-20260810.0 current: release-20260817.0
Five consecutive restores all produced that same clean error, no panic. Environment:
systrap, aarch64, Linux 6.12.x, inside a kind node (Kubernetes 1.35 / containerd 2.2.0),
though the CLI repro above does not involve either.
**Where I did see it go wrong** was restoring through the containerd shim, with a
checkpoint written by a different build and `pages_meta.img` present. There the async
loader reached a bad length before the version check ran, and the sentry died:
panic: runtime error: makeslice: len out of range
...
gvisor.dev/gvisor/pkg/sentry/pgalloc.(*MemoryFile).LoadFrom(...)
pkg/sentry/pgalloc/save_restore.go:1034 +0x1c8
gvisor.dev/gvisor/pkg/sentry/kernel.(*AsyncMFLoader).backgroundGoroutine(...)
pkg/sentry/kernel/kernel_restore.go:366 +0x3f4
created by gvisor.dev/gvisor/pkg/sentry/kernel.NewAsyncMFLoader in goroutine 50
pkg/sentry/kernel/kernel_restore.go:331 +0x120
surfacing to the caller only as the control connection dropping:
FATAL ERROR: starting container: restoring container "d9d4fe8ea273...": urpc method "containerManager.Restore" failed: EOF
Those were binaries built from my own branch, not release builds, so that exact pages
layout is not reproducible from a stock release pair. Presumably those two builds' pages
files diverged enough for `LoadFrom` to read a nonsense length, where 20260810 and
20260817 happen not to. I can't turn that into a repro you can run, so treat it as
context rather than evidence.
The narrow question, then, is just whether starting the pages load before validating
`runsc_version` is intended. Reading the metadata and comparing versions first would mean
a mismatched image is never partially parsed, and the failure is always the clean error
above rather than depending on whether the bytes happen to decode. The metadata read does
not look like it depends on the pages file, but I don't know whether something else in
that ordering matters.
Entirely reasonable to close this if the ordering is deliberate — the version check does
work on release builds, which is the case that matters for most users.
Contributor guide
Research direction
Start in runsc/boot/controller.go at the restore path, then read the runsc_version comparison and the HavePagesFile branch that creates kernel.NewAsyncMFLoader. Trace whether metadata can be read and the version checked before pages loading begins. Done means a mismatched checkpoint consistently returns the clean version error without starting asynchronous pages parsing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100