Fix race condition when switching root manager

Before this change, the root manager package name is only written into
the database after the repackaged APK is installed. In the time between
the repackaged APK being installed and the package name being written
into the database, if some operation calls `get_manager`, the Magisk
daemon will cache this result and ignore the repackaged APK, even if
the package name is set afterwards, because the cache won't be
invalidated. The result is that the repackaged manager APK will not be
recognized as the root manager, breaking the hide manager feature.

This race condition is more likely to happen when Zygisk is enabled,
because `get_manager` is called with a very high frequency in that case.

To fix the issue, we have to set the new package name into the database
BEFORE installing the repackaged APK. We also stop pruning the
database if the repackaged manager is not found, moving this logic into
the Magisk app. By doing so, we can guarantee that the instant after
the repackaged manager APK is installed, the Magisk daemon will
immediately pick it up and treat it as the root manager.

Another small optimization: when the requester is root, simply bypass
the whole database + manager package check. Since the Magisk app hiding
APK installation proces will call `su` several times to run `pm` under
different UIDs, doing this opimization will reduce the amount of
unnecessary database query + filesystem traversals.
This commit is contained in:
topjohnwu 2023-03-06 03:25:59 -08:00
parent 1aade8f8a8
commit a1ce6f5f12
4 changed files with 23 additions and 19 deletions

View File

@ -160,7 +160,6 @@ object HideAPK {
private fun launchApp(activity: Activity, pkg: String) {
val intent = activity.packageManager.getLaunchIntentForPackage(pkg) ?: return
Config.suManager = if (pkg == APPLICATION_ID) "" else pkg
val self = activity.packageName
val flag = Intent.FLAG_GRANT_READ_URI_PERMISSION
activity.grantUriPermission(pkg, Provider.preferencesUri(self), flag)
@ -191,6 +190,7 @@ object HideAPK {
launchApp(activity, pkg)
}
Config.suManager = pkg
val cmd = "adb_pm_install $repack $pkg"
if (Shell.cmd(cmd).exec().isSuccess) return true
@ -239,6 +239,7 @@ object HideAPK {
launchApp(activity, APPLICATION_ID)
dialog.dismiss()
}
Config.suManager = ""
val cmd = "adb_pm_install $apk $APPLICATION_ID"
if (Shell.cmd(cmd).await().isSuccess) return
val success = withContext(Dispatchers.IO) {

View File

@ -134,7 +134,7 @@ abstract class SplashActivity<Binding : ViewDataBinding> : NavigationActivity<Bi
Shell.cmd("(pm uninstall $APPLICATION_ID)& >/dev/null 2>&1").exec()
}
} else {
if (!Const.Version.atLeast_25_0() && Config.suManager.isNotEmpty())
if (Config.suManager.isNotEmpty())
Config.suManager = ""
pkg ?: return
Shell.cmd("(pm uninstall $pkg)& >/dev/null 2>&1").exec()

View File

@ -126,7 +126,7 @@ int get_manager(int user_id, string *pkg, bool install) {
if (access(app_path, F_OK) == 0) {
// Always check dyn signature for repackaged app
if (!mgr_pkg->empty() && !check_dyn(user_id))
goto not_found;
goto ignore;
if (pkg) *pkg = name;
return user_id * AID_USER_OFFSET + mgr_app_id;
} else {
@ -163,7 +163,7 @@ int get_manager(int user_id, string *pkg, bool install) {
// Check the repackaged package name
bool invalid = false;
auto check_pkg = [&](int u) -> bool {
auto check_stub_apk = [&](int u) -> bool {
ssprintf(app_path, sizeof(app_path),
"%s/%d/%s", APP_DATA_DIR, u, str[SU_MANAGER].data());
if (stat(app_path, &st) == 0) {
@ -194,16 +194,16 @@ int get_manager(int user_id, string *pkg, bool install) {
return false;
};
if (check_pkg(user_id)) {
if (check_stub_apk(user_id)) {
if (!check_dyn(user_id))
goto not_found;
goto ignore;
if (pkg) *pkg = *mgr_pkg;
return st.st_uid;
}
if (!invalid) {
collect_users();
for (int u : users) {
if (check_pkg(u)) {
if (check_stub_apk(u)) {
// Found repackaged app, but not installed in the requested user
goto not_found;
}
@ -212,16 +212,13 @@ int get_manager(int user_id, string *pkg, bool install) {
}
}
// Repackaged app not found, remove package from db
rm_db_strings(SU_MANAGER);
// Fallthrough
// Repackaged app not found, fall through
}
// Check the original package name
bool invalid = false;
auto check_pkg = [&](int u) -> bool {
auto check_apk = [&](int u) -> bool {
ssprintf(app_path, sizeof(app_path), "%s/%d/" JAVA_PACKAGE_NAME, APP_DATA_DIR, u);
if (stat(app_path, &st) == 0) {
#if ENFORCE_SIGNATURE
@ -246,14 +243,14 @@ int get_manager(int user_id, string *pkg, bool install) {
return false;
};
if (check_pkg(user_id)) {
if (check_apk(user_id)) {
if (pkg) *pkg = JAVA_PACKAGE_NAME;
return st.st_uid;
}
if (!invalid) {
collect_users();
for (int u : users) {
if (check_pkg(u)) {
if (check_apk(u)) {
// Found app, but not installed in the requested user
goto not_found;
}
@ -271,8 +268,9 @@ int get_manager(int user_id, string *pkg, bool install) {
install_stub();
not_found:
const char *name = mgr_pkg->empty() ? JAVA_PACKAGE_NAME : mgr_pkg->data();
LOGW("pkg: cannot find %s for user=[%d]\n", name, user_id);
LOGW("pkg: cannot find %s for user=[%d]\n",
mgr_pkg->empty() ? JAVA_PACKAGE_NAME : mgr_pkg->data(), user_id);
ignore:
if (pkg) pkg->clear();
return -1;
}

View File

@ -167,8 +167,13 @@ void prune_su_access() {
static shared_ptr<su_info> get_su_info(unsigned uid) {
LOGD("su: request from uid=[%d]\n", uid);
shared_ptr<su_info> info;
if (uid == AID_ROOT) {
auto info = make_shared<su_info>(uid);
info->access = SILENT_SU_ACCESS;
return info;
}
shared_ptr<su_info> info;
{
mutex_guard lock(cache_lock);
if (!cached || cached->uid != uid || !cached->is_fresh())
@ -183,8 +188,8 @@ static shared_ptr<su_info> get_su_info(unsigned uid) {
// Not cached, get data from database
info->check_db();
// If it's root or the manager, allow it silently
if (info->uid == AID_ROOT || to_app_id(info->uid) == to_app_id(info->mgr_uid)) {
// If it's the manager, allow it silently
if (to_app_id(info->uid) == to_app_id(info->mgr_uid)) {
info->access = SILENT_SU_ACCESS;
return info;
}