topjohnwu / Magisk

The Magic Mask for Android
GNU General Public License v3.0
45.54k stars 11.42k forks source link

Flashing Magisk.zip fails on Amlogic slot a-only devices due to slot 'normal' naming convention #6572

Closed finnzz closed 1 year ago

finnzz commented 1 year ago

Device: FireTV 2nd gen Cube Android version: Android 9 (FireOS 7) Magisk version name: Canary 831a398b Magisk version code: 25206 Amlogic s922x SOC, slot-A only, SAR, no super partition device

When trying to flash magisk.zip within TWRP, to patch the boot partition I get the following error:

**********************************
 Magisk 831a398b(25206) Installer 
**********************************
- Current boot slot: normal
! Cannot mount /system
- Unmounting partitions
Updater process ended with ERROR: 1
Error installing zip file '/data/media/0/magisk-canary.zip'
Done processing script file

I think this may be related to some Amlogic devices using the slot label 'normal'. Previously, I was not able to get this device to boot with a patched Magisk boot, because Magisk didn't recognize what slot 'normal' is. https://github.com/topjohnwu/Magisk/issues/5806 In that instance @canyie applied a fix to whitelist 'normal'. I'm wondering if a similar whitelisting is needed in this case?

finnzz commented 1 year ago

Installing Magisk from broken Recovery is not recommended

Hi, thank you for the followup. Do you mean installing Magisk from recovery is broken in general and not recommended? Or do you mean my device's recovery is broken?

I'm fairly sure my issue is related to the Magisk install script not recognizing the slot name 'normal' used by a subset of Amlogic devices, similar to the earlier issue I linked to above. Otherwise, installing firmware from my recovery works without any problems.

finnzz commented 1 year ago

Talking it over with @osm0sis it looks like Amlogic really is just using slot 'normal' naming convention for their a only devices. Here's an example from one of their reference boards:

"echo active_slot: ${active_slot};"\
            "if test ${active_slot} != normal; then "\
                    "setenv bootargs ${bootargs} androidboot.slot_suffix=${active_slot};"\
            "fi;"\
            "if test ${avb2} = 0; then "\
                "if test ${active_slot} = _a; then "\
                    "setenv bootargs ${bootargs} root=/dev/mmcblk0p23;"\
                "else if test ${active_slot} = _b; then "\
                    "setenv bootargs ${bootargs} root=/dev/mmcblk0p24;"\ 

This creates a problem for any Amlogic slot a only devices that return 'normal' for the slot value. For example in Magisk's /scripts/util_functions.sh

mount_partitions() {
  # Check A/B slot
  SLOT=`grep_cmdline androidboot.slot_suffix`
  if [ -z $SLOT ]; then
    SLOT=`grep_cmdline androidboot.slot`
    [ -z $SLOT ] || SLOT=_${SLOT}
  fi
  [ -z $SLOT ] || ui_print "- Current boot slot: $SLOT"

In this case the the script sees slot 'normal' and tries to mount /dev/block/system_normal, which doesn't exist, rather than /dev/block/system that does.

So I propose a similar fix that was applied for Amlogic devices in init, where 'normal' was just ignored https://github.com/topjohnwu/Magisk/issues/5806

Perhaps something along the lines having slot normal replaced by a blank entry:

mount_partitions() {
  # Check A/B slot
  SLOT=grep_cmdline androidboot.slot_suffix
  if [ -z $SLOT ]; then
    SLOT=grep_cmdline androidboot.slot
    if [ "$SLOT" = "normal" ]; then
      SLOT=""
    else
      [ -z $SLOT ]  SLOT=_${SLOT}
    fi
  fi
  [ -z $SLOT ]  ui_print "- Current boot slot: $SLOT"

I think this slot naming convention is also causing direct installs through Magisk Manager to fail on Amlogic slot a only devices. I get the following error:

!Unable to detect target image
!Installation failed

If a similar whitelisting could be added there, it might fix that error as well.

For reference here are the /dev/block partition listings on my device.

brw-------  1 root root          179,   4 2018-12-31 19:00 boot
brw-------  1 root root          179,   1 2018-12-31 19:00 bootloader
brw-------  1 root root          179,  12 2018-12-31 19:00 cache
brw-------  1 root root          179,  13 2018-12-31 19:00 data
brw-------  1 root root          179,   6 2023-01-19 04:33 logo
brw-------  1 root root            7,   0 2018-12-31 19:00 loop0
brw-------  1 root root            7,   8 2018-12-31 19:00 loop1
brw-------  1 root root            7,  16 2018-12-31 19:00 loop2
brw-------  1 root root            7,  24 2018-12-31 19:00 loop3
brw-------  1 root root            7,  32 2018-12-31 19:00 loop4
brw-------  1 root root            7,  40 2018-12-31 19:00 loop5
brw-------  1 root root            7,  48 2018-12-31 19:00 loop6
brw-------  1 root root            7,  56 2018-12-31 19:00 loop7
brw-------  1 root root          179,   7 2018-12-31 19:00 misc
brw-------  1 root root          179,   0 2018-12-31 19:00 mmcblk0
brw-------  1 root root          179,  32 2018-12-31 19:00 mmcblk0boot0
brw-------  1 root root          179,  64 2018-12-31 19:00 mmcblk0boot1
brw-------  1 root root          179,  96 2018-12-31 19:00 mmcblk0rpmb
brw-------  1 root root          179,   9 2018-12-31 19:00 odm
drwxr-xr-x  3 root root                60 2018-12-31 19:00 platform
brw-------  1 root root          179,  11 2018-12-31 19:00 product
brw-------  1 root root          179,   5 2018-12-31 19:00 recovery
brw-------  1 root root          179,   2 2018-12-31 19:00 reserved
brw-------  1 root root          179,  10 2018-12-31 19:00 system
brw-------  1 root root          179,   3 2018-12-31 19:00 tee
brw-------  1 root root          179,   8 2018-12-31 19:00 vendor
drwx------  2 root reserved_disk       40 2018-12-31 19:00 vold
brw-------  1 root root          252,   0 2018-12-31 19:00 zram0

and

raven:/dev/block/platform/ffe07000.emmc/by-name # ls -al
total 0
drwxr-xr-x 2 root root 300 2018-12-31 19:00 .
drwxr-xr-x 3 root root 400 2018-12-31 19:00 ..
lrwxrwxrwx 1 root root  15 2018-12-31 19:00 boot -> /dev/block/boot
lrwxrwxrwx 1 root root  21 2018-12-31 19:00 bootloader -> /dev/block/bootloader
lrwxrwxrwx 1 root root  16 2018-12-31 19:00 cache -> /dev/block/cache
lrwxrwxrwx 1 root root  15 2018-12-31 19:00 data -> /dev/block/data
lrwxrwxrwx 1 root root  15 2018-12-31 19:00 logo -> /dev/block/logo
lrwxrwxrwx 1 root root  15 2018-12-31 19:00 misc -> /dev/block/misc
lrwxrwxrwx 1 root root  14 2018-12-31 19:00 odm -> /dev/block/odm
lrwxrwxrwx 1 root root  18 2018-12-31 19:00 product -> /dev/block/product
lrwxrwxrwx 1 root root  19 2018-12-31 19:00 recovery -> /dev/block/recovery
lrwxrwxrwx 1 root root  19 2018-12-31 19:00 reserved -> /dev/block/reserved
lrwxrwxrwx 1 root root  17 2018-12-31 19:00 system -> /dev/block/system
lrwxrwxrwx 1 root root  14 2018-12-31 19:00 tee -> /dev/block/tee
lrwxrwxrwx 1 root root  17 2018-12-31 19:00 vendor -> /dev/block/vendor
osm0sis commented 1 year ago

Give this a try: https://github.com/topjohnwu/Magisk/actions/runs/4059349503

finnzz commented 1 year ago

Give this a try: https://github.com/topjohnwu/Magisk/actions/runs/4059349503

Thank you @osm0sis, this fixed both problems installing Magisk from TWRP, and installs from within Magisk Manager!

raven:/ # twrp install magisk-debug.apk
Installing zip file '/data/media/0/magisk-debug.apk'
Installing zip file '/data/media/0/magisk-debug.apk'
Unmounting System...
**********************************
 Magisk 987199e6(25206) Installer 
**********************************
- Mounting /system
- Mounting /vendor
- Device is system-as-root
- System-as-root, keep dm/avb-verity
- No vbmeta partition, patch vbmeta in boot image
- Target image: /dev/block/boot
- Device platform: armeabi-v7a
- Constructing environment
- Unpacking boot image
- Checking ramdisk status
- Magisk patched boot image detected
- Patching ramdisk
- Repacking boot image
- Flashing new boot image
- Unmounting partitions
- Done
Done processing script file

I had spent a bit of time problem shooting an issue with the build that I can no longer reproduce after doing a data wipe of the device. So I think everything is good. There was just one minor issue.

Magisk Manager doesn't detect the device's ramdisk (no), even though the installation log shows that the ramdisk was added, and Magisk appears to be fully functional, granting SU to apps. The Cube (perhaps another Amlogic quirk?) is one of those few devices who's boot.img doesn't include a ramdisk, but will use it if it's added. I don't know if this is related.

Magisk Manager 25.2 stable does report (yes) for ramdisk after patching, even if I use Magisk Manager 25.2 in combination with the patched boot image made by this CI v25206 (987199e6). This may just be a superficial issue since Magisk is working anyway.

johnt1989 commented 1 year ago

https://topjohnwu.github.io/Magisk/boot.html

A only device Type III: Late 2018 - 2019 devices that are A-only. The worst type of device to ever exist as far as Magisk is concerned.

As the same as mine, nothing to be worried about just we don't have a ramdisk in the boot image so magisk reports back saying "no"

finnzz commented 1 year ago

https://topjohnwu.github.io/Magisk/boot.html

A only device Type III: Late 2018 - 2019 devices that are A-only. The worst type of device to ever exist as far as Magisk is concerned.

As the same as mine, nothing to be worried about just we don't have a ramdisk in the boot image so magisk reports back saying "no"

Hah, luckily I think these Amlogic devices are more close to: Type I: Good old legacy ramdisk boot

Because although they don't have a boot ramdisk, the boot image can accept and use a ramdisk when added to the boot.img. In the case of Magisk, it adds it's small ramdisk to my boot.img, and the system uses it without any problems. The device does not have to first boot into recovery. Magisk 25.2 stable reports no ramdisk before I patch the boot.img, and they reports yes to ramdisk after boot is patched.

I was told that Amlogic devices typically do have ramdisks in their boot.img, but not in Android 9 because it uses SAR. Maybe this is why my device that is on Android 9 accepts and uses the ramdisk added by Magisk?

osm0sis commented 1 year ago

Just for clarity it is technically Type III, but it's the good kind that will accept a ramdisk when one's added. 🤘

pndwal commented 1 year ago

Hah, luckily I think these Amlogic devices are more close to: Type I: Good old legacy ramdisk boot

Ramdisk=No devices are all Legacy SAR A-only devices... While early SAR Pixel devices launched w/ A7, they were A/B... I believe we only have A9 launch version A-only true SAR devices, ie. Ramdisk=No... A10+ LV devices are all 2SI...

Because although they don't have a boot ramdisk, the boot image can accept and use a ramdisk when added to the boot.img. In the case of Magisk, it adds it's small ramdisk to my boot.img, and the system uses it without any problems. The device does not have to first boot into recovery. Magisk 25.2 stable reports no ramdisk before I patch the boot.img, and they reports yes to ramdisk after boot is patched.

That's interesting... Normally Ramdisk=No won't change even though Magisk 'manually' adds basic ramdisk if it's missing when patching images... The fact is Magisk now always does this just in case an added ramdisk can work (ie. in case bootloader inexplicably supports ramdisk in boot as per most A-only SAR Xiaomi devices) although it should not be assumed that a non factory ramdisk will work...

So there is now actually ramdisk in every Magisk-patched boot image as Magisk adds it! ... But since this DOES NOT mean it's usable, Magisk simply reports if FACTORY ramdisk is present as this is the only one sure to work. (Factory ramdisk indicates compatible bootloader. Manually added ramdisk indicates nothing!)... Johns design ensures that if Magisk indicates Ramdisk=Yes it should work as bootloader definitely supports it...

Software upgrades don't change this either, and (with some exceptions like Xiaomi) the bootloader won't support boot type ramdisk in any case, so Ramdisk=No remains...

So by design (for good reason) the ramdisk flag (in Magisk app) shows the FACTORY state of support for ramdisk in boot image... It will show 'No' despite the presence of ramdisk in recovery also, and even after basic ramdisk is manually added to boot image... As John has stated:

Some Type III devices’ bootloader will still accept and provide initramfs that was manually added to the boot image to the kernel (e.g. some Xiaomi phones), but many device don’t (e.g. Samsung S10, Note 10). It solely depends on how the OEM implements its bootloader.

It is literally impossible for Magisk to know whether a device bootloader accepts ramdisk, so the assumption is to always inform the user to use recovery mode

https://github.com/topjohnwu/Magisk/issues/3239#issuecomment-703260778

Re. change from No to Yes on your device, Did you patch or is it a downloaded patch? (Guessing that a downloaded patch could add cpio randisk extracted from another other stock ROM?)

I was told that Amlogic devices typically do have ramdisks in their boot.img, but not in Android 9 because it uses SAR.

Yup, because LV A9 are all A-only SAR specifically...

Maybe this is why my device that is on Android 9 accepts and uses the ramdisk added by Magisk?

SAR devices are 'stuck' with Legacy SAR; they aren't upgraded to 2SI although Legacy Ramdisk boot-type devices are... So LV A8(-) devices w/ RV (running version) 10+ become 2SI... LV A9 devices on A10+ remain Legacy SAR... Nothing will change if you update to A10+...

'accepting and using the ramdisk added by Magisk' works purely because OEM has inexplicably retained bootloader support for ramdisk in boot as mentioned above whereas most OEMs removed that for A-only SAR as no boot ramdisk is supplied or used in such devices... Perhaps they were lazy or perhaps it was common to adapt A8 bootloader's for A9 and they made only necessary changes...

Either way, you can be thankful for that as it's the only reason you don't need the ugly recovery mode Magisk solution usually required...

finnzz commented 1 year ago

Thank you for the details!

It may be like you said, that the OEM was lazy about modifying the bootloader. The bootloader UART does report sizes for the boot.img kernel, dtb and ramdisk. It just reports the ramdisk size as 0x0, but it does appear to be looking for one.