Fix /data/adb/magisk permissions so Zygisk can initialise

General.install() runs copy() -> extra1() -> set_perm(). set_perm() only walks
self.files, which are paths under copy_dir (the overlay), so the copy that
Magisk.setup() makes into <data_dir>/adb/magisk keeps the 0644 the binaries
inherit from the apk extraction.

A non-executable /data/adb/magisk/magisk64 makes magisk_env() abort at
post-fs-data. Root still works, because that runs from the overlay copy, but
Zygisk never initialises: the Magisk app reports "Zygisk: No" no matter what the
zygisk setting says, and no error is surfaced to explain why. This breaks every
Zygisk module (LSPosed, Shamiko, PlayIntegrityFix, ...).

Fix setup() to apply the same 0755 to its own destination that
Magisk.set_path_perm() already applies to magisk paths.

Adds a regression test that passes with the fix and fails without it. It needs no
Waydroid install, no root and no extra dependencies:

    python3 tests/test_magisk_setup_perms.py

Fixes #283

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pull/284/head
MisterWizzard 4 weeks ago
parent d5289cfd89
commit b09e018894

@ -64,7 +64,28 @@ on property:init.svc.zygote=stopped
Logger.info("Additional setup")
magisk_absolute_dir = os.path.join(self.copy_dir, self.magisk_dir)
data_dir = get_data_dir()
shutil.copytree(magisk_absolute_dir, os.path.join(data_dir, "adb", "magisk"), dirs_exist_ok=True)
data_magisk_dir = os.path.join(data_dir, "adb", "magisk")
shutil.copytree(magisk_absolute_dir, data_magisk_dir, dirs_exist_ok=True)
# General.install() calls set_perm() only over self.files, which are paths
# under copy_dir (the overlay). This /data/adb copy is made here, before
# that runs, so it keeps the 0644 the binaries inherited from the apk
# extraction. A non-executable /data/adb/magisk/magisk64 makes magisk_env()
# abort at post-fs-data: root still works from the overlay copy, but Zygisk
# silently never initialises (Magisk reports "Zygisk: No" no matter what
# the zygisk setting says). Apply the same perms set_path_perm() gives
# magisk paths, to this copy too.
self.set_data_magisk_perm(data_magisk_dir)
def set_data_magisk_perm(self, data_magisk_dir):
if not os.path.isdir(data_magisk_dir):
return
os.chmod(data_magisk_dir, 0o755)
for root, dirnames, filenames in os.walk(data_magisk_dir):
for dirname in dirnames:
os.chmod(os.path.join(root, dirname), 0o755)
for filename in filenames:
os.chmod(os.path.join(root, filename), 0o755)
def copy(self):
magisk_absolute_dir = os.path.join(self.copy_dir, self.magisk_dir)

@ -0,0 +1,179 @@
#!/usr/bin/env python3
"""Regression test for Magisk.setup() permissions on the /data/adb/magisk copy.
install() runs copy() -> extra1() -> set_perm(). set_perm() only walks self.files,
which are paths under copy_dir (the overlay), so the copy that setup() makes into
<data>/adb/magisk keeps the 0644 the binaries inherit from the apk extraction.
A non-executable /data/adb/magisk/magisk64 makes magisk_env() abort at post-fs-data.
Root still works (it runs from the overlay copy), but Zygisk never initialises and
the Magisk app reports "Zygisk: No" with no error explaining why.
Runs as an ordinary user, needs no Waydroid install and no extra dependencies:
python3 tests/test_magisk_setup_perms.py
"""
import os
import shutil
import stat
import sys
import tempfile
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
import stuff.magisk as magisk_module
from stuff.magisk import Magisk
MAGISK_BINARIES = [
"magisk64",
"magiskinit",
"magiskboot",
"magiskpolicy",
"busybox",
"util_functions.sh",
]
def make_overlay(base):
"""Mimic the tree copy() leaves behind: binaries written 0644 by shutil.copyfile."""
magisk_dir = os.path.join(base, "system", "etc", "init", "magisk")
os.makedirs(os.path.join(magisk_dir, "chromeos"), exist_ok=True)
for name in MAGISK_BINARIES:
path = os.path.join(magisk_dir, name)
with open(path, "wb") as f:
f.write(b"\x7fELF placeholder")
os.chmod(path, 0o644)
nested = os.path.join(magisk_dir, "chromeos", "futility")
with open(nested, "wb") as f:
f.write(b"placeholder")
os.chmod(nested, 0o644)
return magisk_dir
def file_modes(root):
modes = {}
for dirpath, _dirnames, filenames in os.walk(root):
for name in filenames:
path = os.path.join(dirpath, name)
modes[os.path.relpath(path, root)] = stat.S_IMODE(os.stat(path).st_mode)
return modes
def test_plain_copytree_loses_exec_bit():
"""Document the original behaviour: a bare copytree leaves the binaries 0644."""
base = tempfile.mkdtemp()
try:
src = make_overlay(base)
dst = os.path.join(base, "data", "adb", "magisk")
os.makedirs(os.path.dirname(dst), exist_ok=True)
shutil.copytree(src, dst, dirs_exist_ok=True)
modes = file_modes(dst)
assert modes["magisk64"] == 0o644, (
"expected the pre-fix behaviour to reproduce, got %o" % modes["magisk64"]
)
finally:
shutil.rmtree(base, ignore_errors=True)
def test_setup_makes_data_copy_executable():
"""setup() must leave every file and dir in <data>/adb/magisk executable."""
base = tempfile.mkdtemp()
try:
make_overlay(base)
data_dir = os.path.join(base, "data")
os.makedirs(data_dir, exist_ok=True)
original_get_data_dir = magisk_module.get_data_dir
original_info = magisk_module.Logger.info
magisk_module.get_data_dir = lambda: data_dir
magisk_module.Logger.info = lambda *a, **k: None
class StubMagisk(Magisk):
@property
def copy_dir(self):
return base
try:
StubMagisk().setup()
finally:
magisk_module.get_data_dir = original_get_data_dir
magisk_module.Logger.info = original_info
dst = os.path.join(data_dir, "adb", "magisk")
modes = file_modes(dst)
assert modes, "setup() copied nothing"
not_executable = {n: oct(m) for n, m in modes.items() if m != 0o755}
assert not not_executable, "files left non-0755: %s" % not_executable
assert stat.S_IMODE(os.stat(dst).st_mode) == 0o755
nested = os.path.join(dst, "chromeos")
assert stat.S_IMODE(os.stat(nested).st_mode) == 0o755, "nested dir not fixed"
finally:
shutil.rmtree(base, ignore_errors=True)
def test_setup_is_idempotent():
base = tempfile.mkdtemp()
try:
make_overlay(base)
data_dir = os.path.join(base, "data")
os.makedirs(data_dir, exist_ok=True)
original_get_data_dir = magisk_module.get_data_dir
original_info = magisk_module.Logger.info
magisk_module.get_data_dir = lambda: data_dir
magisk_module.Logger.info = lambda *a, **k: None
class StubMagisk(Magisk):
@property
def copy_dir(self):
return base
try:
StubMagisk().setup()
StubMagisk().setup()
finally:
magisk_module.get_data_dir = original_get_data_dir
magisk_module.Logger.info = original_info
dst = os.path.join(data_dir, "adb", "magisk")
assert all(m == 0o755 for m in file_modes(dst).values())
finally:
shutil.rmtree(base, ignore_errors=True)
def test_perm_helper_tolerates_missing_dir():
Magisk().set_data_magisk_perm(os.path.join(tempfile.gettempdir(), "definitely-not-here"))
def main():
tests = [
test_plain_copytree_loses_exec_bit,
test_setup_makes_data_copy_executable,
test_setup_is_idempotent,
test_perm_helper_tolerates_missing_dir,
]
failed = 0
for test in tests:
try:
test()
except AssertionError as exc:
failed += 1
print("FAIL %s: %s" % (test.__name__, exc))
except Exception as exc: # noqa: BLE001 - surface anything unexpected
failed += 1
print("ERROR %s: %r" % (test.__name__, exc))
else:
print("ok %s" % test.__name__)
if failed:
print("\n%d test(s) failed" % failed)
return 1
print("\nall %d tests passed" % len(tests))
return 0
if __name__ == "__main__":
sys.exit(main())
Loading…
Cancel
Save