libmicrovmi
libmicrovmi copied to clipboard
Memory as file descriptor
This PR implements the memory as a file descriptor, using Rust standard traits
-
Read
-
Write
-
Seek
A second memory object is implemented to handle "padded" memory reads, useful for Volatility.
Changes
To introduce this memory abstraction, I had to update the API.
The library doesn't return a dyn Introspectable
anymore, but a Microvmi
struct, which contains the driver and both memory objects (padded and non padded).
On this Microvmi
struct, the methods are simply forwarding to the driver implementation.
Regarding the memory objects, I had to use a Rc<Refcell<Box<dyn introspectable>>>
to share the driver between all the structs, allowing a mutable borrow checked at runtime.
I tested against master while dumping the memory and the overhead is invisible.
Some unit tests have been implemented for the seek implementation.
I also changed how the drivers should implement reading physical memory.
The new API is:
fn read_frame(&self, _frame: PageFrame, _buf: &mut [u8]) -> Result<(), IoError> {
unimplemented!();
}
Which allows us to remove complexity from the driver implementation to the Read
trait implementation (especially for Xen), and share this for all drivers.
Codecov Report
Merging #195 (9fef107) into master (acf7b73) will increase coverage by
4.43%
. The diff coverage is19.82%
.
@@ Coverage Diff @@
## master #195 +/- ##
==========================================
+ Coverage 14.19% 18.62% +4.43%
==========================================
Files 5 7 +2
Lines 472 671 +199
Branches 88 108 +20
==========================================
+ Hits 67 125 +58
- Misses 383 513 +130
- Partials 22 33 +11
Flag | Coverage Δ | |
---|---|---|
unittests | 18.62% <19.82%> (+4.43%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
src/api.rs | 1.73% <0.00%> (+1.73%) |
:arrow_up: |
src/capi.rs | 0.00% <0.00%> (ø) |
|
src/driver/kvm.rs | 26.73% <0.00%> (+4.21%) |
:arrow_up: |
src/errors.rs | 0.00% <ø> (ø) |
|
src/lib.rs | 100.00% <ø> (+95.65%) |
:arrow_up: |
src/microvmi.rs | 0.00% <0.00%> (ø) |
|
src/memory.rs | 37.50% <37.50%> (ø) |
|
... and 1 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update acf7b73...9fef107. Read the comment docs.
This PR is ready for review. @rageagainsthepc , if you want to have a quick look.
@Wenzel Will do 👍
Imho the microvmi struct looks a bit overly complicated. I would prefer a design where you (as a library user) initialize the driver like we used to and then pass it to one of the memory abstraction classes:
let drv = driver_init();
let memory = Memory::new(drv);
let padded_memory = Paddedmemory::new(drv);
Not every user needs memory abstractions, but those who do could easily initialize them. But it's just a thought, haven't looked at the PR in detail yet. Maybe I'm not seeing the bigger picture.
Edit: Right now this implementation would force the library user to write single threaded applications (because Rc
does not implement Send
). It would be better if the user could choose between Rc
or Arc
for example. The idea above would allow this behavior with minimal adjustments to the current implementation I think.
Edit2: If you make your memory abstraction classes generic over the trait bound BorrowMut<Box<dyn Introspectable>>
that should do the trick.
Hi @rageagainsthepc and thank you for your feedback.
I should have explained a bit more why I decided this implementation and design when opening the PR.
I would have also have prefered a simpler design, where the library would return a trait object (Box<dyn Introspectable
), and provide wrapper on top of this, (Memory::new(drv)
).
The issues comes from exposing this API in C and Python. because how do you keep the context ?
The C API exposes a void* drv
pointer, to be casted into our box<dyn Introspectable>
, however, this wouldn't retain the Memory
object context as well, since it's a separate object.
That's the main reason why I choose to go this way, keep a single struct Microvmi
to be casted as void*
into the C world, back and forth, and able to offer the read/write/seek abstraction underneath.
A second reason is that we are able to simplify our read implementations in the drivers, mainly the Xen driver, since the main algorithm to iterate over each frame is implemented in the layer above.
And another reason is to be able to implement batch read operation later on, hidden in the Memory object implementation. Memflow has support for this since some of its targets have a very high latency, so have to batch you read operations, otherwise the whole read will become slow and uneffective.
Edit: as a bonus, I just noticed that with the struct Microvmi
we can keep the DriverType
after initialization, and therefore remove get_driver_type()
from the driver's implementation