Skip to content
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

maps: handle the gap behind .text of the exe #124

Merged
merged 1 commit into from Jul 23, 2015

Conversation

sriemer
Copy link
Member

@sriemer sriemer commented Jul 19, 2015

With the 04/2015 Linux kernel commit a87938b2e246
("fs/binfmt_elf.c: fix bug in loading of PIE binaries")
we've noticed that our assumption that nothing gets loaded into
the gap behind the .text region of the executable has proven wrong.
Unrelated regions might be loaded via mmap() to that location.
Scanmem sets the region type to 'misc' instead of 'exe' for the
.rodata, .data and .bss sections of the executable and also the
load address is incorrect this way.

So count the regions belonging to the executable separately and if
there are not at least two of them, don't reset that count and try
to detect further regions belonging to the executable. There must
be at least a .data region. Also remember the load address of the
executable and assign it to the other regions belonging to it.

This has been tested with PIE binaries with and without an affected
kernel. This has been also tested with regular executables without
PIE with and without an affected kernel.

Reference:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a87938b2e246b81b4fb713edb371a9fa3c5c3c86

Fixes: GitHub issue #122

@sriemer
Copy link
Member Author

sriemer commented Jul 21, 2015

@coolwanglu We really have to fix this in scanmem. Due to the gap between .text and .rodata there is always the chance that something else mmap()s unrelated regions to this location.

* variable address used within the binariy with it.
* variable address used within the binary with it.
*
* 04/2015: Usually there is nothing in between these four
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of appending the log, I suggest you to update the existing ones to reflect the current situation.

@coolwanglu
Copy link
Contributor

Just finished reading the relevant text. Frankly speaking I don't 100% understand our algorithm there. It seems that some of our assumptions there were broken by the latest Linux kernel.

I think test cases may be introduced to test this function, just to make sure that its behavior won't change if updated in the future.

@sriemer
Copy link
Member Author

sriemer commented Jul 23, 2015

Just the assumption that nothing else can be loaded between .text and .rodata of the executable has been broken. The gap there is intended behavior. The Linux kernel puts mmap()ed regions into that gap.
How do you imagine test cases?
There are no other test cases. It depends on the Linux kernel and tools which use mmap().

The algorithm works as before. After reading the .text region an unrelated region is read marking the end of the executable. But as we count the regions belonging to the executable now and know that executables without .data don't exist, we check the filename for all unrelated regions loaded into the gap. If we find a second region belonging to the executable by filename, then we continue with the exe processing as if there wouldn't be anything else in the gap.

I have a short test tool for mmap().

@coolwanglu
Copy link
Contributor

@sriemer a test case would be a txt file with the content of an actual maps file we have ever seen. And we should except the function to return the correct load address and number of regions for example.

Could be easier to maintain this way. But it's up to you, I'm thinking about this because I found it not easy to prove the correctness by reading the source code.

@sriemer
Copy link
Member Author

sriemer commented Jul 23, 2015

My test case would be usually taking a game which uses static memory with PIE, finding the variable with scanmem and lock it with ugtrain. That's what I do e.g. with Warzone 2100.

@coolwanglu
Copy link
Contributor

@sriemer All right if you find it easier to test it that way.

It LGTM, feel free to merge it after updating the comments.

Thanks!

@sriemer
Copy link
Member Author

sriemer commented Jul 23, 2015

Thanks! Yes, static memory cheating would be the common use case for it. Otherwise, you wouldn't have to think about the region type detection at all. Doing it with a real world example tests multiple things at once and you read from the actual /proc/$pid/maps file and don't have to maintain a special case where you read from a different file.

Would it be okay for you if I add my copyright for last year and this year as well to that file?
The region type detection is quite unique and worth being under copyright.

@coolwanglu
Copy link
Contributor

Sure!

With the 04/2015 Linux kernel commit a87938b2e246
("fs/binfmt_elf.c: fix bug in loading of PIE binaries")
we've noticed that our assumption that nothing gets loaded into
the gap behind the .text region of the executable has proven wrong.
Unrelated regions might be loaded via mmap() to that location.
Scanmem sets the region type to 'misc' instead of 'exe' for the
.rodata, .data and .bss sections of the executable and also the
load address is incorrect this way.

So count the regions belonging to the executable separately and if
there are not at least two of them, don't reset that count and try
to detect further regions belonging to the executable. There must
be at least a .data region. Also remember the load address of the
executable and assign it to the other regions belonging to it.

This has been tested with PIE binaries with and without an affected
kernel. This has been also tested with regular executables without
PIE with and without an affected kernel.

Reference:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/
commit/?id=a87938b2e246b81b4fb713edb371a9fa3c5c3c86

Fixes: GitHub issue scanmem#122
@sriemer sriemer changed the title maps: support kernels with 04/2015 PIE loading bugfix maps: handle the gap behind .text of the exe Jul 23, 2015
@sriemer
Copy link
Member Author

sriemer commented Jul 23, 2015

@coolwanglu Is it better now? I've only changed the description. I've created a branch "old-pie-fix" in case you want to compare.

@coolwanglu
Copy link
Contributor

Looks good now. Thanks!

coolwanglu added a commit that referenced this pull request Jul 23, 2015
maps: handle the gap behind .text of the exe
@coolwanglu coolwanglu merged commit a2fdaf9 into scanmem:master Jul 23, 2015
@sriemer sriemer deleted the pie-fix branch July 23, 2015 21:03
sriemer referenced this pull request Oct 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants