-
Notifications
You must be signed in to change notification settings - Fork 136
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
LKL MMU support #551
LKL MMU support #551
Conversation
Thank you, @thehajime! Just rebased the changes.
I haven't thought about it yet -- but it does sound very interesting. I wonder what would be the semantics of the fork(2) in LKL? For example is it:
If we are considering case #1 -- then I think it should be doable, in theory... We would need to create (i.e. copy from the parent) stack for the new forked LKL task and I guess the rest should be straightforward unless I'm missing something. For example, somewhat similar approach has been implemented in d781d59 (private LKL branch for fuzzing Android Binder driver). In this case we don't do the full fork of the task but trick kernel in treating LKL tasks as if they were running in different processes. Essentially, the tasks would have different TGID. |
what I have in my mind is definitely 1). I think 2) is also doable but hard to see how to synchronize the (LKL) kernel state between two host processes, like using same IP address between two LKL processes, etc. and thanks for the pointer for your ongoing patch; indeed tweaking the host_task (of LKL) might be a good first step to implement a proper process model under LKL. I saw a similar attempt (below, web page is in Japanese but code should be in English), which I'd like to see in future (wasmlinux project). https://zenn.dev/okuoku/scraps/8c54763b218805 |
This looks great, thanks @rodionov 🎉 ! Could you please rebase to remove the github update commit? Also, could you please add a CI test for the MMU? |
Thanks for @tavip, @thehajime !!
Just fixed the PR to address these points. As a major delta to the previous changes posted earlier: d31e012 is needed to fix virtio implementation and a849840 adding CI integration. |
In its current implementation MMU support is limited to 64-bit LKL configurations. However, nothing limits it to be extended for 32-bit configurations. It is largely inspired by MMU implementation in UML. Currently, we implement 3-level page tables: page global directory, page middle directory and page table entries (while p4ds and puds are folded into pgd level). This enables it to translate 39-bit virtual addresses to physical addresses in flat memory configuration with kernel virtual addresses identity-mapped to the corresponding physical addresses (with the exception of vmalloc-allocated virtual memory). To build LKL with MMU enabled run: ``` make -C tools/lkl MMU=1 clean-conf all ``` Signed-off-by: Eugene Rodionov <[email protected]>
Running `make -C tools/lkl clean-conf` doesn't remove generated kernel configuration (.config and .config.old) as well as generated kernel header files. As a result, subsequent `make -C tools/lkl all` might use .config from the previous build even if arch/lkl/Kconfig is changed. Adding mrproper target to clean-conf fixes this issue. Signed-off-by: Eugene Rodionov <[email protected]>
The two test cases verify that `mmap` works both for `MAP_SHARED` and `MAP_PRIVATE` mappings. It is important to use `MAP_POPULATE` flag when calling `lkl_sys_mmap` syscall. This flag populates the mapping with the pages so that no page fault happens when the mapping is accessed. ``` make -C tools/lkl MMU=1 clean-conf all tools/lkl/test/boot ``` Signed-off-by: Eugene Rodionov <[email protected]>
The test suite implements just a single test case verifying that `vmalloc` works as expected. ``` make -C tools/lkl MMU=1 MMU_KUNIT=1 clean-conf all tools/lkl/tests/boot ``` Signed-off-by: Eugene Rodionov <[email protected]>
If PATH variable contains paths with whitespaces this could lead to build breackage. This change fixes this issue. Signed-off-by: Eugene Rodionov <[email protected]>
Run LKL tests (boot, disk, network) for LKL built with MMU configuration and KASan enabled. Signed-off-by: Eugene Rodionov <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall, LGTM !
I only have a minor question on the patch; what is the motivation to support MMU in this context ? (not giving a objection but just wondering)
anyway, thanks (and wish to have a happy new year).
Thanks a lot, @thehajime!! The main motivation for this patchset is to enable fuzzing Android Binder driver using LKL to find security vulnerabilities. Android Binder depends on MMU configuration and, thus, we needed to fix this to move forward with fuzzing. This deck https://androidoffsec.withgoogle.com/posts/attacking-android-binder-analysis-and-exploitation-of-cve-2023-20938/offensivecon_24_binder.pdf (slides 68-81 specifically) cover results of fuzzing Binder driver with LKL. LKL fuzzer identified two CVEs: CVE-2023-20938 & CVE-2023-21255. Currently, the working version of the fuzzer is hosted in this branch https://github.com/rodionov/lkl/tree/h2hc_binder. Ideally I would like to merge the fuzzer (once the code is cleaned-up) to LKL repo to put next to the existing HID fuzzer https://github.com/lkl/linux/tree/master/tools/lkl/fuzzers/hid. I'll wait for LGTM from @tavip to make sure that both of you are onboard with the changes in this PR. Thanks again! And Happy New Year! :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for the great work @rodionov!
Hi @thehajime, @tavip,
Posting LKL MMU implementation for review. The list of changelists in this PR implement MMU configuration for LKL and add a few unit tests. Current implementation enables LKL for 64-bit targets, however, nothing limits it to extend this functionality to 32-bit LKL targets in future.
LKL MMU implementation relies on host's mmap to map/unmap LKL's physical pages at provided virtual addresses. Hence, the implementation calls the host's mmap with
MAP_FIXED_NOREPLACE
flag to force memory allocation at certain virtual addresses. For that I introduced twoKconfig
variables:LKL_TASK_UNMAPPED_BASE
defaulting to0x100000
-- this is starting address at whichlkl_sys_mmap
should map the memory. I.e. the very first call tolkl_sys_mmap
would return0x100000
if the allocation is successful. The second call would return0x100000 + mmap1_size
and etc.LKL_MEMORY_START
defaulting to0x50000000
-- this is staring address at which LKL kernel virtual memory is allocated. Thus, translation between LKL pa to va is as follows:va = pa + LKL_MEMORY_START
. Some additional considerations: VMALLOC_START (starting virtual address forvmalloc
-allocated kernel virtual memory) and init's stack top are relative toLKL_MEMORY_START
.This change doesn't work yet with the current virtio implementation which assumes that physical addresses provided by virtio LKL driver are valid virtual memory (in no MMU configuration physical address == virtual address ). With MMU enabled physical address != virtual address and, thus, additional handling needs to be implemented in virtio client libs (under tools/lkl/lib) to translate LKL-provided physical addresses to virtual address. I'll post this fix in a subsequent PR.
Would appreciate any comments and feedback! :)
Thanks!