portal: bound the devfs/procfs mmap staging buffer (fix #79)#81
Open
lacraig2 wants to merge 1 commit into
Open
portal: bound the devfs/procfs mmap staging buffer (fix #79)#81lacraig2 wants to merge 1 commit into
lacraig2 wants to merge 1 commit into
Conversation
The devfs and procfs pseudofile proxies seed and flush their shmem backing
through a single contiguous (k)vzalloc() sized to the entire mmap length:
- portal_devfs.c igloo_devfs_proxy_mmap() : kvzalloc(size) seed
- portal_devfs.c igloo_devfs_flush_shm_...() : kvzalloc(size) flush on release
- portal_procfs.c proxy mmap() : kvzalloc(size) seed
The length comes straight from vma->vm_end - vma->vm_start (then the shmem
file's i_size) with no bound. Because the size is guest-controlled this is an
unbounded kernel allocation:
- On 32-bit donor kernels (vmalloc space ~128-240 MB) a large device mmap
fails outright, e.g. a ~784 MB region gives
"vmalloc: allocation failure: 821952512 bytes ... igloo_devfs_proxy_release".
- A guest can repeatedly mmap()+close() a proxied pseudofile to force large
kernel vmalloc allocations on demand.
The shmem backing is created sparse (VM_NORESERVE); only the staging buffer is
the problem. igloo_fetch_mmap_page() already takes a byte offset, so copy the
region in fixed-size (64 KiB, IGLOO_DEVFS_STAGE_SZ) windows instead. Kernel
memory use is now O(1) regardless of mmap length, and large device mmaps work
on 32-bit guests.
No functional change for small mmaps; the seed/flush data path and offsets are
preserved.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #79.
Problem
The devfs and procfs pseudofile proxies seed and flush their
shmembacking through a single contiguous(k)vzalloc()sized to the entire, guest-controlled mmap length:portal_devfs.cigloo_devfs_proxy_mmap()—kvzalloc(size)seedportal_devfs.cigloo_devfs_flush_shm_to_hypervisor()—kvzalloc(size)flush on releaseportal_procfs.cproxymmap()—kvzalloc(size)seedsizeisvma->vm_end - vma->vm_start(then the shmem file'si_size) with no bound. So:mmap()+close()a proxied pseudofile to force large kernelvmallocallocations on demand.__fput, far from the offendingmmap().Fix
The shmem backing is created sparse (
VM_NORESERVE); only the staging buffer is the problem.igloo_fetch_mmap_page()already takes a byte offset, so copy the region in fixed-size 64 KiB windows (IGLOO_DEVFS_STAGE_SZ, added toportal_internal.h) instead of one full-length allocation. Kernel memory is now O(1) regardless of mmap length, and large device mmaps work on 32-bit guests. The seed/flush data path and per-window offsets are preserved, so there's no functional change for small mmaps.Testing
main./dev/sharedmemioctl that yields a ~784 MB mmap length.vmalloc: allocation failure: 821952512 bytes … igloo_devfs_proxy_release→ the mapping process is torn down.__versionsCRC table is byte-identical to the stock release for all imported symbols.