firecracker-microvm / firecracker-microvm/firecracker

bug: discard_range() silently fails to free physical memory for memfd-backed (MAP_SHARED) guest regions

Open
#6,167 1 comment 0 reactions 1 assignee Claimed by @JackThomson2 View on GitHub
Status: Awaiting assignee
Dominant language
Rust
Stars
36.7k
Forks
2.6k
Avg merge
3d 2h
Merged PRs (30d)
53

Description

## Description

`GuestRegionMmapExt::discard_range()` in `src/vmm/src/vstate/memory.rs` calls
`madvise(MADV_DONTNEED)` for all non-private-file-backed mappings, including
memfd-backed regions created with `MAP_SHARED`. However, `madvise(MADV_DONTNEED)`
has **no effect** on `MAP_SHARED` mappings — the kernel does not release the
physical pages because they are part of a shared file and may be accessed by
other mappers. The call returns 0 (success), so there is no error logged and no
indication the discard was silently dropped.

A code comment at line 758 already acknowledges this:

```rust
// TODO: madvise(MADV_DONTNEED) doesn't actually work with memfd
// (or in general MAP_SHARED of a fd). In those cases we should use
// fallocate64(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE).
// We keep falling to the madvise branch to keep the previous behaviour.
```

## Impact

`discard_range()` is called by the balloon device when the guest inflates the
balloon (i.e. surrenders memory pages back to the host). When the guest is
configured with memfd-backed memory — which is the case when huge pages are
enabled (`HugePageConfig::Hugetlbfs2M` / `Hugetlbfs1G`) — inflating the
balloon does **not** release physical memory back to the host. The balloon
device reports success to the guest and to the caller, but host physical
memory is never reclaimed.

In concrete terms: a Firecracker microVM configured with huge pages and a
balloon device cannot use the balloon to reduce its host memory footprint.

## Reproduction

1. Start a microVM with `huge_pages` enabled and a balloon device configured.
2. Inside the guest, inflate the balloon (e.g. via a balloon tool that inflates
to, say, 256 MiB).
3. On the host, observe via `cat /proc//status | grep VmRSS` or
`/proc//smaps` that the host RSS for the Firecracker process does
**not** decrease after balloon inflation.

## Root Cause

`discard_range()` uses `madvise(MADV_DONTNEED)` for the catch-all `_` branch,
which covers both anonymous (`MAP_ANON`) and shared-file (`MAP_SHARED`)
mappings. For anonymous mappings this works correctly. For `MAP_SHARED`
mappings the kernel ignores `MADV_DONTNEED` for shared pages and does nothing.

## Suggested Fix

Add a dedicated branch for shared file mappings and use
`fallocate(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE)` on the backing fd:

```rust
(Some(file_offset), flags) if flags & libc::MAP_SHARED != 0 => {
let offset = file_offset.start() + caddr.raw_value();
// SAFETY: offset and len are within the file bounds.
let ret = unsafe {
libc::fallocate64(
file_offset.file().as_raw_fd(),
libc::FALLOC_FL_PUNCH_HOLE | libc::FALLOC_FL_KEEP_SIZE,
offset as libc::off64_t,
len as libc::off64_t,
)
};
if ret < 0 {
Err(GuestMemoryError::IOError(std::io::Error::last_os_error()))
} else {
Ok(())
}
}
```

`FALLOC_FL_PUNCH_HOLE` creates a hole in the file, causing the kernel to free
the underlying physical pages. This works on both regular files and memfds,
including hugetlbfs-backed memfds.

## Affected Code

- `src/vmm/src/vstate/memory.rs:718–775` — `GuestRegionMmapExt::discard_range()`
- `src/vmm/src/vstate/memory.rs:867–888` — `memfd_backed()`, which creates the MAP_SHARED regions affected by this bug

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.