mmap: drop file-offset checking, defer to `mmap(2)` instead #320

roypat · 2025-03-21T10:23:04Z

When mmap-ing a file-like object, vm-memory was trying to validate the
the range [offset, offset+len) was valid in the file. However, our
homegrown check had lots of edge cases where it gave false-positives
(e.g. it rejected things that should really be working, such as vfio
devices, or fds that cannot be seeked like guest_memfd), and trying to
implement a check that works for all of these is in the end wasted
effort anyway, because the kernel validates all this as part of the mmap
syscall anyway. So just drop these checks in favor of failing at
mmap-time.

Summary of the PR

Please summarize here why the changes in this PR are needed.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

All commits in this PR have Signed-Off-By trailers (with
git commit -s), and the commit message has max 60 characters for the
summary and max 75 characters for each description line.
All added/changed functionality has a corresponding unit/integration
test.
All added/changed public-facing functionality has entries in the "Upcoming
Release" section of CHANGELOG.md (if no such section exists, please create one).
Any newly added unsafe code is properly documented.

vm-memory has faulty validation logic that prevents us from mmap-ing guest_memfds, so just bypass that by calling mmap ourselves for the time being. See also rust-vmm/vm-memory#320 Signed-off-by: Patrick Roy <[email protected]>

When mmap-ing a file-like object, vm-memory was trying to validate the the range [offset, offset+len) was valid in the file. However, our homegrown check had lots of edge cases where it gave false-positives (e.g. it rejected things that should really be working, such as vfio devices, or fds that cannot be seeked like guest_memfd), and trying to implement a check that works for all of these is in the end wasted effort anyway, because the kernel validates all this as part of the mmap syscall anyway. So just drop these checks in favor of failing at mmap-time. Drop a test case about mmap-ing a file of size 0 - this works because the mmap will simply grow the file. Signed-off-by: Patrick Roy <[email protected]>

vm-memory has faulty validation logic that prevents us from mmap-ing guest_memfds, so just bypass that by calling mmap ourselves for the time being. See also rust-vmm/vm-memory#320 Signed-off-by: Patrick Roy <[email protected]>

roypat requested review from alexandruag, bonzini and jiangliu as code owners March 21, 2025 10:23

roypat force-pushed the stat-instead-of-seek branch 3 times, most recently from db86b91 to cbced4a Compare March 21, 2025 10:40

ShadowCurse previously approved these changes Mar 24, 2025

View reviewed changes

roypat mentioned this pull request Mar 26, 2025

MmapRegion::build fails when the backing object is a device #195

Closed

roypat force-pushed the stat-instead-of-seek branch 2 times, most recently from 1ba4642 to d97d9cd Compare March 26, 2025 15:23

roypat changed the title ~~Allow constructing MmapRegions for non-seekable fds~~ mmap: drop file-offset checking, defer to mmap(2) instead Mar 26, 2025

roypat force-pushed the stat-instead-of-seek branch 2 times, most recently from 006c987 to 55dafa9 Compare March 26, 2025 15:51

roypat dismissed ShadowCurse’s stale review via 4005762 April 11, 2025 09:26

roypat force-pushed the stat-instead-of-seek branch from 4005762 to e7518f7 Compare April 16, 2025 06:19

ShadowCurse approved these changes Apr 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mmap: drop file-offset checking, defer to `mmap(2)` instead #320

mmap: drop file-offset checking, defer to `mmap(2)` instead #320

roypat commented Mar 21, 2025 •

edited

Loading

mmap: drop file-offset checking, defer to mmap(2) instead #320

Are you sure you want to change the base?

mmap: drop file-offset checking, defer to mmap(2) instead #320

Conversation

roypat commented Mar 21, 2025 • edited Loading

Summary of the PR

Requirements

mmap: drop file-offset checking, defer to `mmap(2)` instead #320

mmap: drop file-offset checking, defer to `mmap(2)` instead #320

roypat commented Mar 21, 2025 •

edited

Loading