KSU fixes

[zer0def]

Fixes µG permissions and library unpacking by leveraging huge similarities between Magisk's and KernelSU's installation processes. Unfortunately, KSU doesn't use update-binary, but rather customize.sh instead, so I had to copy the script over while working on deduplicating this code in a manner working with systemized, Magisk and KernelSU installs at the same time.

[FriendlyNeighborhoodShane]

Neat, thanks! Are the crashes fixed for you?

Well, see, this is what I meant when I said customise.sh being a shim would be less work :)

"work" might not have been the right word to use, ofc you've done this great job of figuring these changes out. But we wouldn't need to have this duplication that way. I'd rather the customise.sh just be a small script unzipping and calling update-binary, and all the changes to the main script made to properly coexist with magisk/system code.

[FriendlyNeighborhoodShane]

diff --git src/META-INF/com/google/android/update-binary res/util/script-customize.sh
old mode 100644
new mode 100755
index 4bdd779..ce30c23
--- a/tmp/MinMicroG/src/META-INF/com/google/android/update-binary
+++ b/res/util/script-customize.sh
@@ -7,14 +7,16 @@
 # Copyright 2018-2020 FriendlyNeighborhoodShane
 # Distributed under the terms of the GNU GPL v3

+SKIPUNZIP=1
 exec 3>&1;
 exec 1>&2;

 outfd="/proc/self/fd/$2";
-zipfile="$3";
+zipfile="${3:-${ZIPFILE}}";

customise.sh should just pass $ZIPFILE as an argument, so we won't need this.

 ps | grep zygote | grep -v grep >/dev/null && bootmode=true || bootmode=false;
 $bootmode || ps -A 2>/dev/null | grep zygote | grep -v grep >/dev/null && bootmode=true;
+$BOOTMODE && bootmode=true || bootmode=false;

$BOOTMODE will not be set 66% of the time (im great at probabilities) so instead of an error maybe just [ "$BOOTMODE" = true ] && bootmode=true; ?

@@ -470,10 +472,11 @@ fi;

 ui_print " ";
 ui_print "Mounting...";
-if [ -e "/data/adb/magisk" ] && [ "$forcesys" != "yes" ]; then
+if [ -e "/data/adb/magisk" -o -e "/data/adb/ksu" ] && [ "$forcesys" != "yes" ]; then
   rootpart="/data";
-  $bootmode && modulesdir="$rootpart/adb/modules_update" || modulesdir="$rootpart/adb/modules";
-  root="$modulesdir/$modname";
+  modulesdir="${MODPATH%/*}";
+  $bootmode && modulesdir="${modulesdir:-$rootpart/adb/modules_update}" || modulesdir="${modulesdir:-$rootpart/adb/modules}";
+  root="${MODPATH:-${modulesdir}/${modname}}";
   magisk=yes;
   log "Using $modulesdir";
   [ -d "$modulesdir" ] || {

I really don't like the mess of variable substitutions this creates 😬

I'd rather there just be another else if branch for explicit kernelsu checks, since it seems to have different logic anyway (does it have modules_update?).

[FriendlyNeighborhoodShane]

Sorry for the large amount of changes I requested, this is just a bit icky as it stands RN.

[zer0def]

Still running through the gauntlet of tests, but this should be fine™.

[FriendlyNeighborhoodShane]

I added a bunch of comments about this and that. Once those are resolved, looks great to me.

[zer0def]

Uh, those may have gone missing. Could you quickly reiterate?

[FriendlyNeighborhoodShane]

I just meant the review comments. They're still there both in the comments above and on the files page, since the diff for them hasn't changed.

[zer0def]

Uh, the diff is a part of OP comment, not related to the commit itself, so I would need to update the diff in OP for it to be representative of current state, which has become a lot more readable in the Files changed section.

[FriendlyNeighborhoodShane]

The review comments above aren't visible for you? They still show 'Pending' for me.

[zer0def]

I guess I managed to bork Github by amending & force-pushing into my own branch. Spectacular service, that one.

[FriendlyNeighborhoodShane]

I guess not the, I'll just copy everything here:

About update-binary:

1) (older commit)

Ah, this won't do what we want it to. It will just never do a system install unless forced.

What I meant was an independent branch in the outer if itself:

if [ -e "/data/adb/magisk" ] && [ "$forcesys" != "yes" ]; then
    ...
else if [ -e "/data/adb/ksu" ] && [ "$forcesys" != "yes" ]; then
    ...
else

The content in both branches will be the same, except the different paths ofc (I'll probably rename the variable magisk=yes along with other magisk-y assumptions, in a refactor later).

2) (current commit)

Not quite right either. This means no recovery installs for magisk/kernelsu.

I guess you're trying to avoid redundant checks and code, but there's a lot of stuff assuming the system/magisk dichotomy. I'll just have to wrap it into a neat bow later. We can just take the simpler route for now, an independent else if branch for kernelsu.

3)

So this brings up the question: what do you want to do about non-booted kernelsu installations? $MODPATH will, ofc, only be exported in a booted install through the kernelsu app. But kernelsu modules can be installed through the recovery as well, can't they? (the situation with encryption in most recoveries notwithstanding)

If we only want to install through the app, better to just add a check for [ "$BOOTMODE" = true ] in the if condition for the kernelsu branch itself.

If we want recovery support, we need to have a conditional set on $modulesdir for non-booted environments.

[FriendlyNeighborhoodShane]

for customise.sh:

1) We should add a shebang just as good practice.

2) We don't need either the set or the exports; these variables are already exported and will be inherited by a child process. And SKIPUNZIP doesn't need to be exported.

3) handy tip I didn't know when I wrote the autoflash zip: you can use unzip's -j option to "junk" (discard/flatten) paths when extracting, so update-binary will end up directly in the target dir and we won't need to type the entire path.

unzip has no POSIX spec, but this is supported by at least busybox, which is all customise.sh should care about.

4) I don't know what customise.sh's arguments are or whether it even has them, but we don't need any of them since everything is in env. The first argument is never used by update-binary, the second is only used in recovery, and 4th onwards are never even read. So we can just have:

"$TMPDIR/$modname/update-binary" "" "" "$ZIPFILE";

[zer0def]

Per point no 2: that definitely wasn't the case with ${MODPATH}, as it broke spectacularly in testing, so those exports are staying for safety.

[FriendlyNeighborhoodShane]

Ah true, I realise now that the documentation states that the variables are "declared", probably by whatever script sources customise.sh, not exported.

Still, we don't need the set -a. That'll add noise in the environment from the above-mentioned script's variables, could even interfere with functioning.

[zer0def]

Dropped set -a per request.

[FriendlyNeighborhoodShane]

Thanks, customize.sh looks good. Only the update-binary changes remain.

[zer0def]

Uh, correct me if I'm wrong, but isn't $bootmode set only when the script finds a running zygote (implying a booted Android system) or $BOOTMODE is set from Magisk/KernelSU? That would mean that systemless installs (Magisk/KernelSU) occur only on a booted/inited Android, making systemized installs in recovery mode deterministic and fixing a case where a previously installed Magisk creates /data/adb/magisk, potentially tripping a systemless install within recovery whenever userdata is mounted, which then may require forcing as systemized. Not sure whether that's intended behavior.

As for non-booted/non-inited KernelSU module installation in recovery - this might be a tad bit more involving to pull off, as /data/adb/ksu/modules.img would have to be additionally mounted for modules to be installed in there. As subtly hinted by the previous paragraph, I don't think systemless installs should be performed from recovery at all, as they can become confusing and require forcing in the first place. With that said, I'm perfectly open to the contrary, though I haven't bumped into any other OTA, which makes such far-reaching assumptions.

[FriendlyNeighborhoodShane]

Systemless installs to magisk from recovery were intended, yes, if that's what you mean. It was actually the recommended way (until decryption became a much more common issue), since permissions can be properly cleaned up with a decrypted /data. It's not really that complicated either, you just place files the same way you do for booted installs. Systemless-if-possible was the default since the beggining because... I thought systemless was neat? Anyway, I don't see much of a point in changing it these days. I guess flashable zips don't really 'get their hands dirty' with all of these implementation details these days, since all of them are modules anyway.

A misaimed magisk install was a possibility but I never heard of it happening to anybody. You'd have to mess up uninstalling magisk. by e.g. uninstalling the app without going through the uninstall wizard inside it. It's not even much of a concern these days since data decryption is so hard.

Huh, KernelSU uses a loopmount image to store modules? Does it fork a really, really old version of magisk? This is surprising to hear since magisk dropped the image approach and just used a simple directory because of all the headache and fragility it involves.

Anyway, there used to be code in minmicrog to support the magisk image (though that was so long ago the code isn't even in this repo, you'll have to dig out a really old release from tg from the-place-that-shall-not-be-named). But it was really ugly and complicated. So yeah, just ignoring kernelsu installs from recovery is fine by me.

So I'd say what we want there is a separate branch for the kernelsu installation, and since we're only supporting booted installs, the check can simply be checking $KSU instead of bothering with impl-dependent directories.

[zer0def]

Might be that mounting an overlayfs image from kernel space is more sturdy than it used to be from initrd or other ways to overlay - at this point, a bit beyond scope.

Given context, I'll be pushing out a change shortly.

[FriendlyNeighborhoodShane]

Thanks, the patch looks good now.

Does it install fine for you? All installation messages, no crashes, no errors in install log, etc.?

One other thing that just came to me, for a magisk installation, after placing the module files into the modules_update dir ($MODDIR) in a booted install, you also had to place the module.prop into the actual live modules directory and then touch update; so that the magisk app will show an entry for the module with the footnote "Will be installed/updated on next reboot". Does that happen correctly with you for kernelsu with the current code?

[zer0def]

These align with systemized installation in recovery and work fine after a reboot or two for npem to do it's magic. The KernelSU screen installs a smaller package, because it just happens to be the more constrained device of the two, but doesn't change it's working order.

[FriendlyNeighborhoodShane]

The changes look good, thanks!

I amended the commit with some style nitpicks that I felt I shouldn't bother you with.