crit: add features to retrieve memory pages #133

behouba · 2023-06-02T16:00:40Z

The goal of this PR is to add features for collecting memory pages content.
For the now I have added GetMemPages function to retrieve the memory pages of a process. Alongside it, helper functions genMemChunk and getPage have been implemented. The core logic is derived from /~https://github.com/checkpoint-restore/criu/blob/criu-dev/coredump/criu_coredump/coredump.py.

It's still a work in progress, but I would like to submit it for feedback.

PTAL: @adrianreber, @rst0git

Thank you!

codecov · 2023-06-02T16:48:45Z

Codecov Report

Patch coverage: 78.57% and project coverage change: +1.91 🎉

Comparison is base (748b90b) 48.33% compared to head (4c337d0) 50.25%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
+ Coverage   48.33%   50.25%   +1.91%     
==========================================
  Files          23       24       +1     
  Lines        2435     2589     +154     
==========================================
+ Hits         1177     1301     +124     
- Misses       1084     1104      +20     
- Partials      174      184      +10

Impacted Files	Coverage Δ
test/crit/main.go	`52.20% <33.33%> (-9.34%)`	⬇️
crit/mempages.go	`97.24% <97.24%> (ø)`

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

adrianreber · 2023-06-03T07:52:00Z

Thanks for the PR. I am a bit unsure about the design. Returning the content of the pages as a return parameter does not sound right. For large memory areas I would expect that using pointers would be better and would hopefully require less data movement.

Nice to see tests included in the PR. You could try to also pass a couple of wrong parameters to test the error paths as well.

behouba · 2023-06-05T05:56:51Z

I have made a few changes to the previous implementation:

Added a MemoryReader struct to encapsulate common and reusable data.
Modified the GetMemPages function to accept addresses (start and end) for flexibility, enabling the retrieval of specific portions of memory (e.g., mm_arg, mm_env, pagemap addresses etc.).
The GetMemPages function now returns a *bytes.Buffer instead of []byte.

Do you think this is a good direction for ?

crit/mempages.go

crit/mempages_test.go

crit/mempages.go

rst0git

LGTM

This commit adds the MemoryReader struct with methods to retrieve the memory pages associated with a process. This feature enhances the functionality of crit as a package by providing access processes memory pages content. Signed-off-by: Kouame Behouba Manasse <behouba@gmail.com>

This commit introduces a new function `readMemoryPages` that reads the process arguments and environment variables from memory pages and compares them with the `environ` and `cmdline` files of the process. Signed-off-by: Kouame Behouba Manasse <behouba@gmail.com>

adrianreber · 2023-06-16T08:33:33Z

Looks very good. Thanks.

behouba marked this pull request as draft June 2, 2023 16:00

behouba force-pushed the read-memory-pages branch from 380a8f2 to b1c3bdf Compare June 2, 2023 19:00

behouba force-pushed the read-memory-pages branch from b1c3bdf to 61d6e74 Compare June 5, 2023 05:31

behouba force-pushed the read-memory-pages branch from 61d6e74 to 3d59f68 Compare June 8, 2023 02:09