Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Mach0 coredump generation (WIP) - Revised #4407

Closed
wants to merge 1 commit into from

Conversation

0xddom
Copy link
Contributor

@0xddom 0xddom commented Mar 23, 2016

The code for solving the mach0 part of radare#4124 so far.
Is very WIP because I started late to make the microtask and when I started, I started doing it all wrong (see core.c in the commits).

Also, cleaned up the code and removed core.c.

@@ -3130,6 +3130,9 @@ static int cmd_debug(void *data, const char *input) {
case 'e':
r_core_debug_esil (core, input + 1);
break;
case 'g':
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add comments here // "dg" so that grepping for the command is easier?

@radare radare closed this Mar 23, 2016
@radare radare added this to the 0.10.2 milestone Mar 23, 2016
@radare
Copy link
Collaborator

radare commented Mar 23, 2016

wrong window :X sorry

@radare radare reopened this Mar 23, 2016
@radare
Copy link
Collaborator

radare commented Mar 23, 2016

can you squash, rebase and update the PR to fix the merge conflict and allow proper review?

thanks!


#include <mach-o/loader.h>

// TODO: Put this code in an if that checks if the target is a mach kernel.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is mach0 specific, must be implemented in a plugin, or at least in a separate file debug/mach0 or so

@0xddom
Copy link
Contributor Author

0xddom commented Mar 23, 2016

I have removed core.c and moved all the implementation to the native debug plugin.

@alvarofe
Copy link
Contributor

i sent him through telegram some feedback. Looking forward for the PR update in order to see it again and study thoroughly the logic behind core file generation ;)

@0xddom 0xddom force-pushed the core branch 4 times, most recently from 660b3f0 to ae14338 Compare March 24, 2016 21:40
vm_size_t size = 0;
int n = 1;

for(;;) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing space before (

@radare
Copy link
Collaborator

radare commented Mar 29, 2016

Hardcoded path must be core.%d instead of /cores/core.%d. the user can change hte path with the cd command

@radare
Copy link
Collaborator

radare commented Mar 29, 2016

Use eprintf instead of printf for debuggging messages

@radare
Copy link
Collaborator

radare commented Mar 29, 2016

The final core generated:

  • it is about 1.8GB for /bin/ls
  • can't be loaded with lldb
  • r2 shows as empty mach0 core file

@radare
Copy link
Collaborator

radare commented Mar 29, 2016

The file generation part must be done in RCore, and using the r_sandbox apis. the code in the RDebug plugin must return an RBuffer for that purpose.

@radare
Copy link
Collaborator

radare commented Mar 31, 2016

ping

@0xddom
Copy link
Contributor Author

0xddom commented Mar 31, 2016

I will continue tomorrow

@radare
Copy link
Collaborator

radare commented Apr 1, 2016

Starting to look good!

@0xddom
Copy link
Contributor Author

0xddom commented Apr 2, 2016

There are some bugs here and there. It's not finished.

return -1;
}

if (fchown (fd, info->uid, info->gid) != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need to fail if ownership change fails. it's ok to save the file as r2 the user running r2. in fact, changing it can be problematic for the user in some environments

@radare
Copy link
Collaborator

radare commented Apr 2, 2016

👍

@0xddom
Copy link
Contributor Author

0xddom commented Apr 2, 2016

ACK the feedback. I hope to finish it tomorrow

@radare
Copy link
Collaborator

radare commented Apr 3, 2016

oops. can you squash and drop that merge?

@0xddom
Copy link
Contributor Author

0xddom commented Apr 3, 2016

I will describe here the bug with realloc: during r_buf_append_buf, the dest buf [b] is a pointer to the file by mmap, realloc says that the pointer is not allocated (maybe because is alloacted by mmap and not malloc, so some internal state is missing?)

@radare
Copy link
Collaborator

radare commented Apr 4, 2016

mmapped memory cannot be reallocated. so looks like a bug in r_buf_append_buf, because it should be checking if its malloc or mmaped buf.

@0xddom
Copy link
Contributor Author

0xddom commented Apr 4, 2016

What function should I use for reallocating the buffer in case is a mmap one? mremap is not standard and is not in Darwin. (Also looks like is not very secure)

@@ -1259,6 +1259,52 @@ static int r_debug_desc_native_open (const char *path) {
return 0;
}

static int r_debug_setup_ownership (int fd, RDebug *dbg) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer RDebug don't touch the filesystem at all. and, as long as r_debug_info can be called from RCore you can just fchown in there.

@radare
Copy link
Collaborator

radare commented Apr 4, 2016

i have commited the new RBuffer API

@radare
Copy link
Collaborator

radare commented Apr 5, 2016

Great! :D it works,it can be better (like everything) but it's by far, ready to be merged.

Good work and thanks for the contrib!

@radare radare closed this Apr 5, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants