Fixed the crash on iOS 15 perfectly: changing vm prot according to wh… #87
Conversation
…ether having VM_PROT_WRITE rather than being const seg.
|
I tried, but still crash |
…e the changing scope.
I push a new commit just now, please try again. |
|
The XNU based systems seem having some issues internally, the __DATA_CONST segment should have read only attribute, but some iOS/MacOSX system libraries might write it too. The vm_region/vm_region_64 API reports this segment having only VM_PROT_READ protection attribute, but it is able to be written on some machine and not on some others. |
This is OK. Will this affect the hook function? Includes all iOS versions. |
All the hook functionalities will work fine just like before across all iOS versions. Just as I mentioned in a previous message, some iOS systems have internal Bugs when dealing with vm prot/vm max prot, but the second commit is OK for all these iOS versions. |
| * 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); |
There was a problem hiding this comment.
We are calling vm_protect with same parameters in a loop, can't we call this method outside loop (just once)?
There was a problem hiding this comment.
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. :-)
|
It works for me in macOS 12 beta |
| @@ -81,12 +81,13 @@ static int prepend_rebindings(struct rebindings_entry **rebindings_head, | |||
| return 0; | |||
| } | |||
|
|
|||
| static vm_prot_t get_protection(void *sectionStart) { | |||
| #if 0 | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Please see my comments in the following question thread.
| if (oldProtection & VM_PROT_EXECUTE) { | ||
| protection |= PROT_EXEC; | ||
| } | ||
| mprotect(indirect_symbol_bindings, section->size, protection); |
There was a problem hiding this comment.
With this removed, the section will now be writable forever.
There was a problem hiding this comment.
Yes, you are right! If we keep the codes of writing back previous vm prot value, some iOS/MacOSX system libraries, such as AVFoundation, AudioToolbox etc might lead to crash, keep in mind that these hooking codes will be executed every time for each dlopen calls.
There was a problem hiding this comment.
Is this a threading issue (a race condition)? If so, wouldn't surrounding the block with a mutex help?
There was a problem hiding this comment.
Considering the dlopen being invoked in multithreads scenario, yes there is a race condition, but we could not resolve this race condition via a mutex completely, because some other codes can change the vm prot directly via vm_protect function too! If fishhook is the only module which would invoke vm_protect to change the vm prot, then a mutex could help, but it is not.
There was a problem hiding this comment.
OK that makes sense! But then if there are race conditions happening, there would also be nothing stopping another different implementation from applying protection after we unprotected the region, thus causing a write to that location to crash in our code.
There's actually a lot of code in fishhook that would break in horrible ways if accessed by multiple threads. A mutex would at least stop fishhook from stomping all over itself.
There was a problem hiding this comment.
Yeah, you are right. But if we just leave the changed memory writable after 'writing operation', seems this is the simplest way, and without many side effects.
|
It works for me in iOS 14.7.1 (iPhone 12) |
问题描述：方法的调用链不能根据 callDepth 来判断，不然所有层级相等的深度，都在一个调用链路中了。 解决方案：通过 lr 作为方法调用的标识，作为方法调用链更加合理。 新增：方法的测试代码验证 lr 的合理性。 问题可以参考：ming1016/RSSReader#30 bugfix2: 问题描述：iOS15及以上的系统运行崩溃 解决方案：更新 fishhook 代码解决，详细问题参考 facebook/fishhook#87
…ether having VM_PROT_WRITE rather than being const seg.