Add required Samsung framework changes to the bridge #59
Comments
wanam
changed the title
wanam
changed the title
|
Can you be a bit more specific? What are you trying to hook, how does it fail (e.g. any exceptions)? By the way, I came across some older attempts of mine to get ART working on Samsung. Some of the enhancements might be useful for you and arter, so I'll send you an email. |
|
I appreciate your help. I'm trying to hook some native mdpp checks to disable it, here are my hooks: http://pastebin.com/9LCU2Wp1 If put those hook on the "initForZygote", it fails with a not found crypto digests, i don't know when they are loaded, i tried to load my hooks on a module while loading the package "android" or "com.android.org.conscrypt" but it seems to late as the hooks are never fired again. |
What's the exact exception you get? Does it fail when you try to hook the methods, when the hook is executed or as a side-effect of the hook being called? Just some comments on your code: The hook for For the constructor hook, you should use |
|
Thanks for tips, I'm running now my hooks on initForZygote before and out of "systemMain" hook and it seems to run better now (updated my changes: http://pastebin.com/9LCU2Wp1). The "not found crypto implementation" was caused by:
i commented this line as it seems this algo implementation is not loaded yet or not existing (logs:http://pastebin.com/hyQFMNGz):
I need some time to figure out what's wrong with those algos, but it seems the hooks on the bridge are working well. Thanks for your help. |
|
I managed to hook all required methods for both files "core-libart.jat" and "conscrypt.jar" with no issues (my changes: http://pastebin.com/9LCU2Wp1), but i still have 2 problems to get rid of the modified "core-libart.jat": 1- I need to add a declaration for 2 additional fields on "java.lang.DexCache":
1- I need to support some Samsung native methods at low level, my knowledge on low level devs is limitted, so i just need some help with the declaration of bellow native methods on android_art, just an empty declaration just to satisfy the link as i will hook them later:
Thanks again for you help. |
|
I have already implemented the native methods regarding reflection some time ago. I will send you the files, but my requirement would be that the changes required for Samsung ROMs will be open-source. I believe that this is one of reasons why Xposed is so successful and also you wouldn't have been able to create your fork without it. |
|
Great, of course all my changes will be open source. Thank you. |
|
Ok, I was just wondering because https://github.com/wanam/android_art seems to contain just one Samsung-related commit. I made way more changes to libart and it still didn't work because it can't decode .odex.xz files (yet?). Also, I remember that you had various issues and crashes while getting it to run, but don't see how those were solved. Anyway, here are my changes, based on AOSP 5.0: https://www.dropbox.com/sh/cj59tyovuuf2cpm/AACIp4qxxNRHUxH_W18PwGD0a?dl=0
As you can see, the intention was to build one set of libraries that will work on Samsung and non-Samsung ROMs, including completely stock (odex'ed) ROMs. Not sure if that's realistic, as @arter97 has merged many more commits. We would have to review their necessity and impact on other ROMs, but I have very limited time to work on Xposed for the few next weeks. My attempts to make Xposed compatible with Samsung stopped at the point where .odex.xz and .art files would have to be used. The .art file might be ignored (maybe?), so I hope this won't be much harder than supporting .odex.gz files, which I hope to finish relatively soon. Anyway, it would be a big step forward even if the ROM just had to be de-odex'ed, but not changed otherwise. Please keep me updated how things work out. |
|
odex.gz support can be found at CyanogenMod, maybe you can take a look at. You can do xz stuffs later(as I don't see that as an emergency development-breaker). And yes, none of my other commits are relavant than String.clear and ignoring arguments. |
|
And as I mentioned to @wanam, you need to take more core-libart.jar and rip it open. So far I can test with S4, Note 3, S5, S6 and Note 5. |
Only for patchoat, not for dex2oat. But yes, I'm basing my work on those commits, which are actually unmerged contributions to AOSP by Intel. So with what you said, it sounds like a universal version might be possible - even it might be weeks or months ahead. Great! |
|
And btw, art-fd is not the only flag you should ignore. S series, Note series, A series, J series.. My decision to include core-libart.jar has solved most of the fragmentation, thankfully. I was actually quite surprised to see no one complaining major issues such as boot. |
|
@arter97 i think the flags are not a big problem, since we can ignore them, but some devices may have additional native methods like "Runtime_appStartupBegin/End" that @rovo89 already added on his patches, those methods exist on some devices only (and not on 5.1.1 builds as i know). @rovo89 Thanks for the patches, i will see how i can use them and report back. |
|
Well, basically I do ignore them meanwhile: rovo89/android_art@afd9bd1 Sure, the variants might be a problem. But as long as they're just there or not, an existence check (or ignoring NoSuchMethodException) would be easy to do. |
|
Plus, I can almost swear I saw backwards-incompatible libcore(core-libart.jar) changes which I don't see it'll be easy to cope both scenario with single binary. In any cases, I'm not here to drag you guys down. Just a friendly "FYI" :) |
|
It's all about trying to minimize (if we can't eliminate) hard-coded changes on the framework, i hate playing with Smali, it was my main reason of switching to xposed, let's hope :) . |
|
Ok, it seems to boot fine with stock framework files and @rovo89 patches :) I didn't use the oat_format patch, i don't know if we need it with a deodexed Rom. |
|
Cool! I assume your other changes from http://pastebin.com/9LCU2Wp1 are still required, minus those that are now implemented natively? And the PathClassLoader implementation is just empty? If I had known that, I could have published those patches half a year ago, but I got stuck with the xz decoding (as I was looking for a solution that works on stock) and also wasn't sure how many other issues would occur. By the way, is the call to restoreUnsupportedServices() really needed when the services aren't removed before? And do you know why these FIPS/MDPP-related methods are incompatible with AOSP ART? Is the implementation for nativeCheckWhitelist() missing or more?
I would have thought so, as boot.oat is precompiled on every ROM as far as I know. Or does deodexing move the individual dex files back to their jar? |
|
Oh, and as I mentioned before: The utility method probably has to be improved. Not sure how to handle ROMs that have a modified core-libart.jar. They might not work correctly with these patches, not sure. So maybe a bitmask or different return values are required to keep it all compatible. |
|
Indeed i removed my native hooks on ArtField/ArtMethod/Class/PathClassLoader. But i kept my mdpp/fips hooks, as they are still required, i have no idea why the Checkmdpp/nativeCheckWhitelist/CheckFipsMode are crashing, there are no helpful logs, they were main cause of the boot.art crashes i had in the past, the logs i was getting were just misleading. The call to restoreUnsupportedServices() is required after the constructor because these services are not default inserted, they are marked as unsupported on the constructor. I will give a try to see what happens on odexed Rom if get some free time this weekend. Edit: the new method on PathClassLoader returns null to force the use of the default Aosp PathClassLoader constructor on the framework side. |
|
So i tested the same xposed version with oat_format patch on Odexed OH2 (5.1.1) and got a boot-loop with a strange permission error: http://pastebin.com/2bBPt0Xu. |
|
The mdpp stuffs are related to Knox. It somehow checks ART binary and rejects it if it's not stock. |
|
@wanam I have seen such permission errors before. The system tries to load the file, but sees that there's no compiled file yet, so it calls dex2oat. Problem is that it can only do that with the current UID and permissions (system_server in this case), not via installd which runs as root. So the missing permission is for writing to the Dalvik cache. Anyway, the fact that it has to compile at this point is just a consequence of a previous failure to compile the same file. Can you check if there's another dex2oat call for this file? Also, are these files compressed? |
|
It says (full logs: http://pastebin.com/w3HmzK7A):
The odex files are not compressed. |
|
Btw guys, where should I be looking at for latest Samsung sources you guys are working on? |
|
I committed my Bridge changes: wanam@eca7063 Some specific Note5 hooks are included in my commit, but still untested (i don't own the device to test them). I didn't make any other changes to android_art, i will commit @rovo89 patches to my repo (https://github.com/wanam/android_art) soon. |
|
Now I'm catching up with the party. Now it seems like all I can help is go check some cross-device's compatibility :) |
|
@wanam Most likely, you will also need to add a check here (maybe restructuring the lines added for Xposed a bit): https://github.com/rovo89/android_art/blob/xposed-lollipop-mr1/dex2oat/dex2oat.cc#L1543 @arter97 Yes, odex files need to be recompiled, otherwise they're too optimized and many hooks won't work or are unreliable (e.g. because some methods will be inlined or called directly). |
|
I'm surprised that odex actually can be recompiled lol. |
Normally, they can't be recompiled, this is something that I have added for Xposed. They contain the dex file, but the code is pre-optimized (just like the odex files on Dalvik).
Not sure what "odex formatting recognition" means. If you're referring to the issue with your boot.oat - that's exactly caused by the optimized code, which contains exactly two statements that ART's method verifier can't handle because it's an edge case. The format can be read just fine and all methods except for this one can be recompiled. I'm working on a work-around for your issue, but that's no reason to stop working on other frontiers. This could actually happen on any other ROM as well, given that the code is complex enough. |
|
I just tried it and it seems to boot fine on odexed OH8, but with a low space issue :) , i will give an other try with the interpret-only filter. |
|
Tried the interpret-only filter with no success, dalvik-cache went down 600MB, but still getting the "not enough space for sustem"! No idea what is triggering this message. |
I don't know this either... But good to know that it works in general. Now we just need to know in which cases the files have to be renamed... |
|
That space issue actually happened to me when messing around the compiler On Wed, Sep 9, 2015 at 4:45 PM, rovo89 [email protected] wrote:
|
|
@arter97 i don't think so, the same patch works fine with my deodexed OH8, and it's not generating any errors on the xposed logs, it fixes by the way the gms bug rovo89/android_art#15. But yes maybe it's my C coding to be blamed, the last time i C codded was more than 10 years ago :) , here are my changes : diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 74a692d..d732037 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -660,7 +660,7 @@ static size_t OpenDexFiles(const std::vector<const char*>& dex_filenames,
continue;
}
std::unique_ptr<File> file;
- if (EndsWith(dex_filename, ".oat") || EndsWith(dex_filename, ".odex")) {
+ if (EndsWith(dex_filename, ".oat") || EndsWith(dex_filename, ".odex") || EndsWith(dex_filename, ".sam.xposed")) {
file.reset(OS::OpenFileForReading(dex_filename));
if (file.get() == nullptr) {
LOG(WARNING) << "Failed to open file '" << dex_filename << "': " << strerror(errno);
@@ -1542,7 +1542,7 @@ static int dex2oat(int argc, char** argv) {
if (dex_filenames.empty()) {
odex_filename = DexFilenameToOdexFilename(zip_location, instruction_set);
if (!OS::FileExists(odex_filename.c_str())) {
- odex_filename += ".gz.xposed";
+ odex_filename = GetRenamedOdexFileName(odex_filename);
}
if (OS::FileExists(odex_filename.c_str())) {
LOG(INFO) << "Using '" << odex_filename << "' instead of file descriptor";
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 5fb3082..96d7d49 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -626,7 +626,9 @@ bool ClassLinker::GenerateOatFile(const char* dex_filename,
std::string odex_filename(DexFilenameToOdexFilename(dex_filename, kRuntimeISA));
std::string dex_file_option("--dex-file=");
- dex_file_option += OS::FileExists(odex_filename.c_str()) ? odex_filename : dex_filename;
+ const char* android_data = GetAndroidData();
+ const std::string dalvik_cache_root(StringPrintf("%s/dalvik-cache/", android_data));
+ dex_file_option += (StartsWith(oat_cache_filename, dalvik_cache_root.c_str()) && OS::FileExists(odex_filename.c_str())) ? odex_filename : dex_filename;
std::string oat_fd_option("--oat-fd=");
StringAppendF(&oat_fd_option, "%d", oat_fd);
diff --git a/runtime/utils.cc b/runtime/utils.cc
index adc701f..120dd00 100644
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -1398,4 +1398,18 @@ bool IsSamsungROM() {
return result;
}
+std::string GetRenamedOdexFileName(std::string odex_filename) {
+ std::string filename (odex_filename + ".gz.xposed");
+ if (OS::FileExists(filename.c_str())) {
+ return filename;
+ } else {
+ filename = odex_filename + ".sam.xposed";
+ if (OS::FileExists(filename.c_str())) {
+ return filename;
+ }
+ }
+
+ return odex_filename;
+}
+
} // namespace art
diff --git a/runtime/utils.h b/runtime/utils.h
index c2bf395..7c1d7d9 100644
--- a/runtime/utils.h
+++ b/runtime/utils.h
@@ -559,6 +559,9 @@ using UniqueCPtr = std::unique_ptr<T, FreeDelete>;
bool IsSamsungROM();
+// Returns renamed odex file name if exists
+std::string GetRenamedOdexFileName(std::string odex_filename);
+
} // namespace art
#endif // ART_RUNTIME_UTILS_H_ |
|
@rovo89 I think there are only two cases for now for the renaming, check if odexes are gz compressed (and maybe xz later, this case should support TW and others), if not then check if TW. |
|
I'm getting this error:
and this one:
Just before the low space error:
So i guess the last one is just a side effect as @arter97 said. Logs: https://www.dropbox.com/s/9j3gmoegmsmqa23/log-odex-low-space.tar.gz?dl=0 |
|
@arter97 I could recompile your boot.oat after this commit: rovo89/android_art@c6341dc @wanam I hope this will also fix the other issues... but I don't really see the relation. |
|
Great i will the new commits soon. |
|
I tried your v74 commits on odexed OH8, the boot.art dexopting seems to be fixed, but i'm getting 2 new errors with SystemUI and AdvSoundDetector2015 with a low space at the end:
and:
Here are the failed files: https://www.dropbox.com/s/oc07yc3x2rqc1z1/AdvSound%2BSystemUI.tar.gz?dl=0 Here are the logs: https://www.dropbox.com/s/r2yzkg7jd1e10vd/log11092015.txt.tar.gz?dl=0 |
|
@rovo89 i replaced both failed files (SystemUI and AdvSoundDetector2015) with a deodexed ones, just verify if they are the real source of this issue, unfortunately no. I still get this error:
|
|
Probably this is off-topic, but starting with Android 5.1, Samsung decided to preopt apks also in /system/container, which I don't think AOSP can understand. Should we need to worry about that, or is it outside of where Xposed is touching? |
|
I don't think it's something we should worry about, we can remove them anyway if we can't use nor need them. |
|
I got many low space reports on some Deodexed TW Roms where the Rom maker left odexed framework inside and they got it fixed by removing arm/arm64 folders, that's why i'm convinced this issue is caused by ReadSpecificImageHeader failing at https://github.com/rovo89/android_art/blob/xposed-lollipop-mr1/runtime/gc/space/image_space.cc#L382 :
I checked the image header magic/version of "boot.art" and it seems correct: "art\n12\0". @rovo89 Do you have any idea what may cause this issue? should i debug those elements verification? https://github.com/rovo89/android_art/blob/xposed-lollipop-mr1/runtime/image.cc#L52 |
|
I did a few more debugging and found that the image header fails exactly at https://github.com/wanam/android_art/blob/xposed-lollipop-mr1/runtime/image.cc#L95 This has been tested without Knox apps nor built-in Facebook, Microsoft ones. |
|
@rovo89 here are the offsets i'm getting with odexed OH8: I think image_roots_ is the wrong one, it should be and offset between image_begin and oat_file_begin_. |
|
Maybe Samsung stores additional information in the image, just like they do for oat files (wanam/android_art@4358b59)? We could either try to find out more details, or try to avoid this check completely. Can you try to change https://github.com/wanam/android_art/blob/xposed-lollipop-mr1/runtime/runtime.cc#L693 to |
|
Yes, forcing must_relocate_ = false fixes this issue without the need to ignore the check. |
|
OK. This will obviously need good testing on 5.0 and 5.1 to ensure that there are no side-effects, especially after clearing the Dalvik cache. I'll add this change locally. |
|
Class.getPublicFieldRecursive() is now implemented as a native method in AOSP: https://android.googlesource.com/platform/art/+/72f9075e92cfc4632d6c50f282a0f6319b718882%5E!/ |
|
As you said there are a lot of differences between the master branch and LL releases, but at least i will try to modify your implementation to follow their search order: current class, interfaces, then super classes. What do you think about this? static mirror::ArtField* getPublicFieldRecursive(mirror::Class* c, const StringPiece& name) {
while (c != nullptr) {
// Search current class
mirror::ArtField* result = getDeclaredFieldInternal(c, name);
if (result != nullptr && result->IsPublic())
return result;
// Search iftable which has a flattened and uniqued list of interfaces
int32_t iftable_count = c->GetIfTableCount();
mirror::IfTable* iftable = c->GetIfTable();
for (int32_t i = 0; i < iftable_count; i++) {
mirror::ArtField* result = getPublicFieldRecursive(iftable->GetInterface(i), name);
if (result != nullptr && result->IsPublic())
return result;
}
// We don't try the superclass if we are an interface.
if(c->IsInterface()){
break;
}
// Get the next class.
c = c->GetSuperClass();
}
return nullptr;
} |
|
I tried the above modification and it seems to work great, but i can't confirm if it has any performance improvements. I actually still have 2 "minor" issues with stock TW 5.1.1 Roms:
Its odex file is an arm64 file, but the dex2oat is executed somehow with the instruction-set arm, so it can't locate the file in the correct folder, Samsung bug? any idea how can i handle it? can i check just the second folder if not found in the first one? https://github.com/wanam/android_art/blob/xposed-lollipop-mr1/dex2oat/dex2oat.cc#L1667 2- Compilation error on SystemUI bundled support-v4:
SystemUI odex file: https://www.dropbox.com/s/i5kqim4w3j307iw/systemui-odex-arm64.zip?dl=0 |
|
I think the point (1) on my above comment does not need to be fixed on the Xposed level, i renamed the odex folder "AdvSoundDetector2015/arm64" to "AdvSoundDetector2015/arm" and it got compiled with no issues, this app has a depending lib32 native libraries too, so i guess it's just a FW packaging bug on the Samsung side. For the second point, it seems related to a RETURN_OBJECT verification for arrays, i will try this: https://android.googlesource.com/platform/art/+/16f149c2cb43a14d8f33d7d0fa36cd784e900f07 |
|
Sadly this modification (https://android.googlesource.com/platform/art/+/16f149c2cb43a14d8f33d7d0fa36cd784e900f07) didn't help much, taking a look at http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/5.0.1_r1/android/support/v4/app/NotificationCompat.java#2114, the return type is "android.support.v4.app.NotificationCompat$Action[]" i don't know why it is expecting the extended class "android.support.v4.app.NotificationCompatBase$Action[]". I committed also a modification wanam/android_art@6e564a6 of "getPublicFieldRecursive" that seems to work great here. |
|
Closing this issue as most of the issues with Samsung Roms have been fixed. |
|
Please make XPosed for Touchwiz happen <3 |
Hi @rovo89 ,
I think you are aware of the unofficial port of Xposed to the Galaxy S6: http://forum.xda-developers.com/xposed/unofficial-xposed-samsung-lollipop-t3180960.
To get this to work with Aosp Art, i made some changes to "core-libart.jar" and "conscrypt.jar", i'm thinking about porting my changes on the XposedBridge to avoid changing both files.
As a first step i'm taking "consrypt.jar" as example, i'm just hooking some methods on it to disable some security checks (mdpp and some white-listing) and add some missing crypto algos, this way i will avoid overriding the whole file on Samsung Roms, if this is possible can you point me where/when i should implement those hooks?
I tried inside "de.robv.android.xposed.initForZygote" and at the end of "handleBindApplication" but this does not seems to work.
Thanks.
The text was updated successfully, but these errors were encountered: