From d0c5f515c05d05c9d24971695337daf9d6ce409c Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Thu, 7 Jun 2012 16:53:59 -0700 Subject: [PATCH] Fix issue #6295373: "Package com.google.android.apps.authenticator2 has... ...mismatched uid: X on disk, Y in settings" errors on Froyo and Gingerbread Deal more gracefully with the uid changing in three ways: 1. If the uid on disk has become root, then have installd change it to the application's uid. This is to correct a potential case where installd was interrupted while linking or unlinking the libs dir, during which it temporarily changes the owner of the dir to root so that a malicious app can not get in its way. So if the uid on disk has become root, we assume we can safely just change it back to the correct uid. 2. When scaning packages at boot, use the same "delete and rebuild data directory" code for third party applications as we have for system applications. This allows us to at least end up in a state where the app will run, even if its data is lost. 3. But we really don't want to get in to case 2, so if an application update is being installed and we find that the uid we now have for the app is different than the one on disk, fail the update. This will protect against for example a developer changing the sharedUserId of their app and getting into this bad state. Bug: 6295373 Change-Id: Ic802fdd818ac62449ff3c61d1fff1aa4d4942f39 --- cmds/installd/commands.c | 41 ++++++++++++++++++- cmds/installd/installd.c | 6 +++ cmds/installd/installd.h | 1 + .../android/content/pm/PackageManager.java | 8 ++++ .../java/com/android/server/pm/Installer.java | 11 +++++ .../server/pm/PackageManagerService.java | 37 ++++++++++++++--- 6 files changed, 96 insertions(+), 8 deletions(-) diff --git a/cmds/installd/commands.c b/cmds/installd/commands.c index 203d180a6439..0bc737185b91 100644 --- a/cmds/installd/commands.c +++ b/cmds/installd/commands.c @@ -106,6 +106,43 @@ int renamepkg(const char *oldpkgname, const char *newpkgname) return 0; } +int fix_uid(const char *pkgname, uid_t uid, gid_t gid) +{ + char pkgdir[PKG_PATH_MAX]; + struct stat s; + int rc = 0; + + if ((uid < AID_SYSTEM) || (gid < AID_SYSTEM)) { + ALOGE("invalid uid/gid: %d %d\n", uid, gid); + return -1; + } + + if (create_pkg_path(pkgdir, pkgname, PKG_DIR_POSTFIX, 0)) { + ALOGE("cannot create package path\n"); + return -1; + } + + if (stat(pkgdir, &s) < 0) return -1; + + if (s.st_uid != 0 || s.st_gid != 0) { + ALOGE("fixing uid of non-root pkg: %s %d %d\n", pkgdir, s.st_uid, s.st_gid); + return -1; + } + + if (chmod(pkgdir, 0751) < 0) { + ALOGE("cannot chmod dir '%s': %s\n", pkgdir, strerror(errno)); + unlink(pkgdir); + return -errno; + } + if (chown(pkgdir, uid, gid) < 0) { + ALOGE("cannot chown dir '%s': %s\n", pkgdir, strerror(errno)); + unlink(pkgdir); + return -errno; + } + + return 0; +} + int delete_user_data(const char *pkgname, uid_t persona) { char pkgdir[PKG_PATH_MAX]; @@ -950,7 +987,7 @@ int linklib(const char* dataDir, const char* asecLibDir) out: if (chmod(dataDir, s.st_mode) < 0) { ALOGE("failed to chmod '%s': %s\n", dataDir, strerror(errno)); - return -errno; + rc = -errno; } if (chown(dataDir, s.st_uid, s.st_gid) < 0) { @@ -1027,7 +1064,7 @@ int unlinklib(const char* dataDir) out: if (chmod(dataDir, s.st_mode) < 0) { ALOGE("failed to chmod '%s': %s\n", dataDir, strerror(errno)); - return -1; + rc = -1; } if (chown(dataDir, s.st_uid, s.st_gid) < 0) { diff --git a/cmds/installd/installd.c b/cmds/installd/installd.c index c2c749ace793..fa4b8a6a1749 100644 --- a/cmds/installd/installd.c +++ b/cmds/installd/installd.c @@ -57,6 +57,11 @@ static int do_rename(char **arg, char reply[REPLY_MAX]) return renamepkg(arg[0], arg[1]); /* oldpkgname, newpkgname */ } +static int do_fixuid(char **arg, char reply[REPLY_MAX]) +{ + return fix_uid(arg[0], atoi(arg[1]), atoi(arg[2])); /* pkgname, uid, gid */ +} + static int do_free_cache(char **arg, char reply[REPLY_MAX]) /* TODO int:free_size */ { return free_cache((int64_t)atoll(arg[0])); /* free_size */ @@ -141,6 +146,7 @@ struct cmdinfo cmds[] = { { "rmdex", 1, do_rm_dex }, { "remove", 2, do_remove }, { "rename", 2, do_rename }, + { "fixuid", 3, do_fixuid }, { "freecache", 1, do_free_cache }, { "rmcache", 1, do_rm_cache }, { "protect", 2, do_protect }, diff --git a/cmds/installd/installd.h b/cmds/installd/installd.h index 78342bb6ac09..1b843fd997ba 100644 --- a/cmds/installd/installd.h +++ b/cmds/installd/installd.h @@ -143,6 +143,7 @@ char *build_string3(char *s1, char *s2, char *s3); int install(const char *pkgname, uid_t uid, gid_t gid); int uninstall(const char *pkgname, uid_t persona); int renamepkg(const char *oldpkgname, const char *newpkgname); +int fix_uid(const char *pkgname, uid_t uid, gid_t gid); int delete_user_data(const char *pkgname, uid_t persona); int make_user_data(const char *pkgname, uid_t uid, uid_t persona); int delete_persona(uid_t persona); diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java index bcdd01273115..6de69b034acf 100644 --- a/core/java/android/content/pm/PackageManager.java +++ b/core/java/android/content/pm/PackageManager.java @@ -519,6 +519,14 @@ public abstract class PackageManager { */ public static final int INSTALL_FAILED_PACKAGE_CHANGED = -23; + /** + * Installation return code: this is passed to the {@link IPackageInstallObserver} by + * {@link #installPackage(android.net.Uri, IPackageInstallObserver, int)} if + * the new package is assigned a different UID than it previously held. + * @hide + */ + public static final int INSTALL_FAILED_UID_CHANGED = -24; + /** * Installation parse return code: this is passed to the {@link IPackageInstallObserver} by * {@link #installPackage(android.net.Uri, IPackageInstallObserver, int)} diff --git a/services/java/com/android/server/pm/Installer.java b/services/java/com/android/server/pm/Installer.java index 9b1973e2825a..48004bbf4151 100644 --- a/services/java/com/android/server/pm/Installer.java +++ b/services/java/com/android/server/pm/Installer.java @@ -243,6 +243,17 @@ class Installer { return execute(builder.toString()); } + public int fixUid(String name, int uid, int gid) { + StringBuilder builder = new StringBuilder("fixuid"); + builder.append(' '); + builder.append(name); + builder.append(' '); + builder.append(uid); + builder.append(' '); + builder.append(gid); + return execute(builder.toString()); + } + public int deleteCacheFiles(String name) { StringBuilder builder = new StringBuilder("rmcache"); builder.append(' '); diff --git a/services/java/com/android/server/pm/PackageManagerService.java b/services/java/com/android/server/pm/PackageManagerService.java index 88f5ec52f8dc..488250824e1d 100644 --- a/services/java/com/android/server/pm/PackageManagerService.java +++ b/services/java/com/android/server/pm/PackageManagerService.java @@ -191,6 +191,7 @@ public class PackageManagerService extends IPackageManager.Stub { static final int SCAN_NO_PATHS = 1<<5; static final int SCAN_UPDATE_TIME = 1<<6; static final int SCAN_DEFER_DEX = 1<<7; + static final int SCAN_BOOTING = 1<<8; static final int REMOVE_CHATTY = 1<<16; @@ -924,7 +925,7 @@ public class PackageManagerService extends IPackageManager.Stub { // Set flag to monitor and not change apk file paths when // scanning install directories. - int scanMode = SCAN_MONITOR | SCAN_NO_PATHS | SCAN_DEFER_DEX; + int scanMode = SCAN_MONITOR | SCAN_NO_PATHS | SCAN_DEFER_DEX | SCAN_BOOTING; if (mNoDexOpt) { Slog.w(TAG, "Running ENG build: no pre-dexopt!"); scanMode |= SCAN_NO_DEX; @@ -3750,17 +3751,34 @@ public class PackageManagerService extends IPackageManager.Stub { } else { // This is a normal package, need to make its data directory. dataPath = getDataPathForPackage(pkg.packageName, 0); - + boolean uidError = false; - + if (dataPath.exists()) { + // XXX should really do this check for each user. mOutPermissions[1] = 0; FileUtils.getPermissions(dataPath.getPath(), mOutPermissions); // If we have mismatched owners for the data path, we have a problem. if (mOutPermissions[1] != pkg.applicationInfo.uid) { boolean recovered = false; - if ((parseFlags&PackageParser.PARSE_IS_SYSTEM) != 0) { + if (mOutPermissions[1] == 0) { + // The directory somehow became owned by root. Wow. + // This is probably because the system was stopped while + // installd was in the middle of messing with its libs + // directory. Ask installd to fix that. + int ret = mInstaller.fixUid(pkgName, pkg.applicationInfo.uid, + pkg.applicationInfo.uid); + if (ret >= 0) { + recovered = true; + String msg = "Package " + pkg.packageName + + " unexpectedly changed to uid 0; recovered to " + + + pkg.applicationInfo.uid; + reportSettingsProblem(Log.WARN, msg); + } + } + if (!recovered && ((parseFlags&PackageParser.PARSE_IS_SYSTEM) != 0 + || (scanMode&SCAN_BOOTING) != 0)) { // If this is a system app, we can at least delete its // current data so the application will still work. int ret = mInstaller.remove(pkgName, 0); @@ -3769,7 +3787,9 @@ public class PackageManagerService extends IPackageManager.Stub { // Remove the data directories for all users sUserManager.removePackageForAllUsers(pkgName); // Old data gone! - String msg = "System package " + pkg.packageName + String prefix = (parseFlags&PackageParser.PARSE_IS_SYSTEM) != 0 + ? "System package " : "Third party package "; + String msg = prefix + pkg.packageName + " has changed from uid: " + mOutPermissions[1] + " to " + pkg.applicationInfo.uid + "; old data erased"; @@ -3781,7 +3801,7 @@ public class PackageManagerService extends IPackageManager.Stub { pkg.applicationInfo.uid); if (ret == -1) { // Ack should not happen! - msg = "System package " + pkg.packageName + msg = prefix + pkg.packageName + " could not have data directory re-created after delete."; reportSettingsProblem(Log.WARN, msg); mLastScanError = PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE; @@ -3794,6 +3814,11 @@ public class PackageManagerService extends IPackageManager.Stub { if (!recovered) { mHasSystemUidErrors = true; } + } else if (!recovered) { + // If we allow this install to proceed, we will be broken. + // Abort, abort! + mLastScanError = PackageManager.INSTALL_FAILED_UID_CHANGED; + return null; } if (!recovered) { pkg.applicationInfo.dataDir = "/mismatched_uid/settings_" -- GitLab