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

Add LOKI support #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add LOKI support #11

wants to merge 1 commit into from

Conversation

xakep666
Copy link

@xakep666 xakep666 commented Jan 23, 2017

add "lokiaboot" and "lokipatch" partition properties to fstab
patch partitions marked as "lokipatch" before flashing

@xakep666 xakep666 force-pushed the master branch 2 times, most recently from 65e0669 to 3c1b210 Compare January 23, 2017 11:13
@M1cha
Copy link
Member

M1cha commented Jan 23, 2017

did you test this?
Since the app is not running as root, it shouldn't be able to read the data of the aboot partition directly.

@xakep666
Copy link
Author

It uses existing backup code to read aboot partition and place it dump to esp. Loki reads this file, not partition.

@M1cha
Copy link
Member

M1cha commented Jan 23, 2017

oh but you're writing that temporary backup to the ESP even though the partition is not being changed(it's information is just used to patch boot/recovery).

You should put the backup in the apps temporary folder instead and delete it when you're done.

@xakep666 xakep666 force-pushed the master branch 2 times, most recently from 1fdcc4c to e67fd8e Compare January 23, 2017 13:01
@xakep666
Copy link
Author

Made a dumping bootloader to getCacheDir() and removing when installation finishes

@@ -105,6 +105,26 @@ public boolean isMultiboot() {
return false;
}

public boolean isLokiAboot() {
Copy link
Member

Choose a reason for hiding this comment

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

the name should be 'isLokiABoot' because A is for Android and B is for Bootloader.


String file = updateDir + "/" + entry.getName() + ".img";

//if needed, apply LOKI patch to boot and recovery partitions
Copy link
Member

Choose a reason for hiding this comment

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

add a space after //

RootToolsEx.dd(file, entry.getBlkDevice());
}
} finally {
//remove bootloader dump
Copy link
Member

Choose a reason for hiding this comment

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

add a space after //

@@ -161,13 +176,36 @@ private void doInstall(String updateDir) throws Exception {
}
}

// install
//dump bootloader to temporary folder
Copy link
Member

Choose a reason for hiding this comment

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

add a space after //

}

public static boolean flashImage(PartitionLabel label, String image) {
//root required for flashing
Copy link
Member

Choose a reason for hiding this comment

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

add a space after //


String mLabel;
PartitionLabel(String s) {
mLabel=s;
Copy link
Member

Choose a reason for hiding this comment

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

space before and after =

private LokiTool() {}

private static native int lokiPatch(String partitionLabel, String bootloaderImage,
String inImage, String outImage);
Copy link
Member

Choose a reason for hiding this comment

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

this linebreak looks weird

private static native int lokiFlash(String partitionLabel, String image);

public static boolean patchImage(PartitionLabel label, String bootloaderImage,
String inImage, String outImage) {
Copy link
Member

Choose a reason for hiding this comment

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

this linebreak looks weird

@@ -114,6 +118,17 @@ private String downloadUpdate(String urlString) throws Exception {
return downloadDir;
}

private static void lokiPatchAndFlash(LokiTool.PartitionLabel partition,
String bootloaderImage, String fileToPatch) throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

this linebreak looks weird

String file = updateDir + "/" + entry.getName() + ".img";

//if needed, apply LOKI patch to boot and recovery partitions
if (entry.isLokiPatch() &&
Copy link
Member

Choose a reason for hiding this comment

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

pls use nested if's to remove the duplicate condition 'isLokiPatch'

Copy link
Member

@M1cha M1cha left a comment

Choose a reason for hiding this comment

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

you should run AndroidStudio's auto-formatting tool on all the files which would make the changes I've requested much easier.

@xakep666 xakep666 force-pushed the master branch 4 times, most recently from d5bb80e to 9e95e7f Compare January 23, 2017 15:26
@xakep666
Copy link
Author

xakep666 commented Jan 23, 2017

Improved encapsulation in "partitionlabel" enum, fixed formatting

Copy link
Member

@M1cha M1cha left a comment

Choose a reason for hiding this comment

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

see my comments

// root required for flashing
boolean isRootAccess = RootTools.isAccessGiven();
if (!isRootAccess) {
Log.e(TAG, "flashImage: cannot get root privileges");
Copy link
Member

Choose a reason for hiding this comment

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

Sry that I missed this previously, but why are you using the volley logtag here?
Also why are you checking for root in first place?

  1. it has already been done at this point
  2. the call to lokiFlash doesn't need root.

Copy link
Author

Choose a reason for hiding this comment

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

About tag: autocompleted at "loge"

Copy link
Author

Choose a reason for hiding this comment

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

Root required because of native library writes new boot and recovery parts

Copy link
Member

Choose a reason for hiding this comment

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

and where does this happen? NDK-code does not run as root and cannot write to any partitions.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

well unless your ROM uses super insecure permissions and selinux is disabled this line will always fail.

Copy link
Author

Choose a reason for hiding this comment

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

You mean I have to use RootToolsEx.dd() to flash patched parts

Copy link
Member

Choose a reason for hiding this comment

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

either that or you have to compile loki as a statically linked binary like busybox and run it as root.
So, didn't you test your change yet? Because such a mistake should have been noticed.

Copy link
Author

Choose a reason for hiding this comment

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

I have no device to test this tool. Also, at first idea was compiling loki_tool. Now I think, maybe just place precompiled binary into "assets" and call it

@xakep666
Copy link
Author

Rewrote using separate binary (runs on my device)

@xakep666 xakep666 force-pushed the master branch 2 times, most recently from 0ed76f6 to 57272fa Compare January 23, 2017 22:17
try {
InputStream is = null;
try {
is = context.getAssets().open("armeabi-v7a/loki_tool_static");
Copy link
Member

@M1cha M1cha Jan 24, 2017

Choose a reason for hiding this comment

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

this binary isn't statically linked. Linking it dynamically actually isn't such a bad idea because it makes the file size much smaller. But what about compatibility? we need to support 4.0+.
Also, is it possible to build binaries with gradle+ndk? The only reason I'm using busybox prebuilt is that it doesn't work that well when compiled using the ndk and still needs to be built using autoconf anyway.

Either way, we don't include the link-type in the filename.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't find way to build and add binary to apk using gradle.
I built it with ndk-build which uses Android.mk included in sources.

Copy link
Member

@M1cha M1cha left a comment

Choose a reason for hiding this comment

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

see my comments

abis.add(Build.CPU_ABI2);
}
// do not initialize on non-arm cpu
if (!abis.contains("armeabi-v7a")) return;
Copy link
Member

Choose a reason for hiding this comment

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

You should rather make the original code for busybox reusable, also don't limit this to ARM.
If you do this you can also keep this code inside RootToolsEx since this implementation shouldn't have to care about where the binary comes from.

Copy link
Author

Choose a reason for hiding this comment

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

This tool is usable only for arm-based phones.
See https://github.com/efidroid/modules_loki/blob/master/loki_patch.c#L30

@xakep666 xakep666 force-pushed the master branch 2 times, most recently from da42dcd to 24cb84c Compare February 14, 2017 09:19
FSTabEntry: getFfMgrFlags() returns list of flags. Rewrite isMultiboot(), isUEFI(), getESP() using it

Move ABIs retrieving code to separate method getABIs() in Util

Add universal image patching interface
Add image patching (if needed) before flashing.
Add LOKI as git submodule
Add LOKI image patcher using NDK
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.

2 participants