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.

Sign up for GitHub

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

Fixed the crash on iOS 15 perfectly: changing vm prot according to wh… #87

Merged
merged 2 commits into from
Oct 12, 2021
Merged

Fixed the crash on iOS 15 perfectly: changing vm prot according to wh… #87

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6da9db8
Fixed the crash on iOS 15 perfectly: changing vm prot according to wh…
haolianfu Jun 11, 2021
31e5182
Moved the vm protection modifying codes inside the condition to reduc…
haolianfu Jun 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 23 additions & 35 deletions fishhook.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ static int prepend_rebindings(struct rebindings_entry **rebindings_head,
return 0;
}

#if 0
Copy link

Choose a reason for hiding this comment

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

Why is this #if 0 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The get_protection function is useless now, because if we change back the vm prot according to its' previous value after the vm 'write operation' done, some iOS/MacOSX system libraries might crash.

Copy link

Choose a reason for hiding this comment

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

Can you explain further? This sounds very strange. If the protection is returned to what it was before the write, wouldn't it be in exactly the state that the rest of the application expects it to be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see my comments in the following question thread.

static int get_protection(void *addr, vm_prot_t *prot, vm_prot_t *max_prot) {
mach_port_t task = mach_task_self();
vm_size_t size = 0;
Expand Down Expand Up @@ -108,6 +109,7 @@ static int get_protection(void *addr, vm_prot_t *prot, vm_prot_t *max_prot) {

return -1;
}
#endif

static void perform_rebinding_with_section(struct rebindings_entry *rebindings,
section_t *section,
Expand All @@ -117,33 +119,6 @@ static void perform_rebinding_with_section(struct rebindings_entry *rebindings,
uint32_t *indirect_symtab) {
uint32_t *indirect_symbol_indices = indirect_symtab + section->reserved1;
void **indirect_symbol_bindings = (void **)((uintptr_t)slide + section->addr);
vm_prot_t prot;
vm_prot_t max_prot;
uintptr_t prot_addr;

if (get_protection(indirect_symbol_bindings, &prot, &max_prot) < 0)
return;

if ((prot & VM_PROT_WRITE) == 0) {
kern_return_t err;

prot_addr = (uintptr_t)indirect_symbol_bindings;
/**
* if this segment does not have the 'write' permission,
* then add it.
* -- Lianfu Hao(@agora.io) Jun 11th, 2021
**/
err = vm_protect (mach_task_self (), prot_addr, section->size, false, prot | VM_PROT_WRITE | VM_PROT_COPY);
if (err != 0) {
/**
* Once we failed to change the vm protection, we
* MUST NOT continue the following write actions!
* iOS 15 has corrected the const segments prot.
* -- Lianfu Hao(@agora.io) Jun 11th, 2021
**/
return;
}
}

for (uint i = 0; i < section->size / sizeof(void *); i++) {
uint32_t symtab_index = indirect_symbol_indices[i];
Expand All @@ -157,23 +132,36 @@ static void perform_rebinding_with_section(struct rebindings_entry *rebindings,
struct rebindings_entry *cur = rebindings;
while (cur) {
for (uint j = 0; j < cur->rebindings_nel; j++) {
if (symbol_name_longer_than_1 &&
strcmp(&symbol_name[1], cur->rebindings[j].name) == 0) {
if (cur->rebindings[j].replaced != NULL &&
indirect_symbol_bindings[i] != cur->rebindings[j].replacement) {
if (symbol_name_longer_than_1 && strcmp(&symbol_name[1], cur->rebindings[j].name) == 0) {
kern_return_t err;

if (cur->rebindings[j].replaced != NULL && indirect_symbol_bindings[i] != cur->rebindings[j].replacement)
*(cur->rebindings[j].replaced) = indirect_symbol_bindings[i];

/**
* 1. Moved the vm protection modifying codes to here to reduce the
* changing scope.
* 2. Adding VM_PROT_WRITE mode unconditionally because vm_region
* API on some iOS/Mac reports mismatch vm protection attributes.
* -- Lianfu Hao Jun 16th, 2021
**/
err = vm_protect (mach_task_self (), (uintptr_t)indirect_symbol_bindings, section->size, 0, VM_PROT_READ | VM_PROT_WRITE | VM_PROT_COPY);
Copy link

Choose a reason for hiding this comment

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

We are calling vm_protect with same parameters in a loop, can't we call this method outside loop (just once)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved vm_protect codes inside loop just for reducing the change scope, i.e. only changing the vm prot when needed. Please keep in mind, although the vm_protect was moved inside the loop, but it will be invoked only the condition being satisfied, so do not think this would be executed more times. :-)

if (err == KERN_SUCCESS) {
/**
* Once we failed to change the vm protection, we
* MUST NOT continue the following write actions!
* iOS 15 has corrected the const segments prot.
* -- Lionfore Hao Jun 11th, 2021
**/
indirect_symbol_bindings[i] = cur->rebindings[j].replacement;
}
indirect_symbol_bindings[i] = cur->rebindings[j].replacement;
goto symbol_loop;
}
}
cur = cur->next;
}
symbol_loop:;
}

if ((prot & VM_PROT_WRITE) == 0)
vm_protect (mach_task_self (), prot_addr, section->size, true, max_prot);
}

static void rebind_symbols_for_image(struct rebindings_entry *rebindings,
Expand Down