Skip to content

Commit d0c5f51

Browse files
author
Dianne Hackborn
committed
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
1 parent 0646411 commit d0c5f51

File tree

6 files changed

+96
-8
lines changed

6 files changed

+96
-8
lines changed

cmds/installd/commands.c

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,43 @@ int renamepkg(const char *oldpkgname, const char *newpkgname)
106106
return 0;
107107
}
108108

109+
int fix_uid(const char *pkgname, uid_t uid, gid_t gid)
110+
{
111+
char pkgdir[PKG_PATH_MAX];
112+
struct stat s;
113+
int rc = 0;
114+
115+
if ((uid < AID_SYSTEM) || (gid < AID_SYSTEM)) {
116+
ALOGE("invalid uid/gid: %d %d\n", uid, gid);
117+
return -1;
118+
}
119+
120+
if (create_pkg_path(pkgdir, pkgname, PKG_DIR_POSTFIX, 0)) {
121+
ALOGE("cannot create package path\n");
122+
return -1;
123+
}
124+
125+
if (stat(pkgdir, &s) < 0) return -1;
126+
127+
if (s.st_uid != 0 || s.st_gid != 0) {
128+
ALOGE("fixing uid of non-root pkg: %s %d %d\n", pkgdir, s.st_uid, s.st_gid);
129+
return -1;
130+
}
131+
132+
if (chmod(pkgdir, 0751) < 0) {
133+
ALOGE("cannot chmod dir '%s': %s\n", pkgdir, strerror(errno));
134+
unlink(pkgdir);
135+
return -errno;
136+
}
137+
if (chown(pkgdir, uid, gid) < 0) {
138+
ALOGE("cannot chown dir '%s': %s\n", pkgdir, strerror(errno));
139+
unlink(pkgdir);
140+
return -errno;
141+
}
142+
143+
return 0;
144+
}
145+
109146
int delete_user_data(const char *pkgname, uid_t persona)
110147
{
111148
char pkgdir[PKG_PATH_MAX];
@@ -950,7 +987,7 @@ int linklib(const char* dataDir, const char* asecLibDir)
950987
out:
951988
if (chmod(dataDir, s.st_mode) < 0) {
952989
ALOGE("failed to chmod '%s': %s\n", dataDir, strerror(errno));
953-
return -errno;
990+
rc = -errno;
954991
}
955992

956993
if (chown(dataDir, s.st_uid, s.st_gid) < 0) {
@@ -1027,7 +1064,7 @@ int unlinklib(const char* dataDir)
10271064
out:
10281065
if (chmod(dataDir, s.st_mode) < 0) {
10291066
ALOGE("failed to chmod '%s': %s\n", dataDir, strerror(errno));
1030-
return -1;
1067+
rc = -1;
10311068
}
10321069

10331070
if (chown(dataDir, s.st_uid, s.st_gid) < 0) {

cmds/installd/installd.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ static int do_rename(char **arg, char reply[REPLY_MAX])
5757
return renamepkg(arg[0], arg[1]); /* oldpkgname, newpkgname */
5858
}
5959

60+
static int do_fixuid(char **arg, char reply[REPLY_MAX])
61+
{
62+
return fix_uid(arg[0], atoi(arg[1]), atoi(arg[2])); /* pkgname, uid, gid */
63+
}
64+
6065
static int do_free_cache(char **arg, char reply[REPLY_MAX]) /* TODO int:free_size */
6166
{
6267
return free_cache((int64_t)atoll(arg[0])); /* free_size */
@@ -141,6 +146,7 @@ struct cmdinfo cmds[] = {
141146
{ "rmdex", 1, do_rm_dex },
142147
{ "remove", 2, do_remove },
143148
{ "rename", 2, do_rename },
149+
{ "fixuid", 3, do_fixuid },
144150
{ "freecache", 1, do_free_cache },
145151
{ "rmcache", 1, do_rm_cache },
146152
{ "protect", 2, do_protect },

cmds/installd/installd.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ char *build_string3(char *s1, char *s2, char *s3);
143143
int install(const char *pkgname, uid_t uid, gid_t gid);
144144
int uninstall(const char *pkgname, uid_t persona);
145145
int renamepkg(const char *oldpkgname, const char *newpkgname);
146+
int fix_uid(const char *pkgname, uid_t uid, gid_t gid);
146147
int delete_user_data(const char *pkgname, uid_t persona);
147148
int make_user_data(const char *pkgname, uid_t uid, uid_t persona);
148149
int delete_persona(uid_t persona);

core/java/android/content/pm/PackageManager.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,14 @@ public NameNotFoundException(String name) {
519519
*/
520520
public static final int INSTALL_FAILED_PACKAGE_CHANGED = -23;
521521

522+
/**
523+
* Installation return code: this is passed to the {@link IPackageInstallObserver} by
524+
* {@link #installPackage(android.net.Uri, IPackageInstallObserver, int)} if
525+
* the new package is assigned a different UID than it previously held.
526+
* @hide
527+
*/
528+
public static final int INSTALL_FAILED_UID_CHANGED = -24;
529+
522530
/**
523531
* Installation parse return code: this is passed to the {@link IPackageInstallObserver} by
524532
* {@link #installPackage(android.net.Uri, IPackageInstallObserver, int)}

services/java/com/android/server/pm/Installer.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,17 @@ public int rename(String oldname, String newname) {
243243
return execute(builder.toString());
244244
}
245245

246+
public int fixUid(String name, int uid, int gid) {
247+
StringBuilder builder = new StringBuilder("fixuid");
248+
builder.append(' ');
249+
builder.append(name);
250+
builder.append(' ');
251+
builder.append(uid);
252+
builder.append(' ');
253+
builder.append(gid);
254+
return execute(builder.toString());
255+
}
256+
246257
public int deleteCacheFiles(String name) {
247258
StringBuilder builder = new StringBuilder("rmcache");
248259
builder.append(' ');

services/java/com/android/server/pm/PackageManagerService.java

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ public class PackageManagerService extends IPackageManager.Stub {
191191
static final int SCAN_NO_PATHS = 1<<5;
192192
static final int SCAN_UPDATE_TIME = 1<<6;
193193
static final int SCAN_DEFER_DEX = 1<<7;
194+
static final int SCAN_BOOTING = 1<<8;
194195

195196
static final int REMOVE_CHATTY = 1<<16;
196197

@@ -924,7 +925,7 @@ public PackageManagerService(Context context, boolean factoryTest, boolean onlyC
924925

925926
// Set flag to monitor and not change apk file paths when
926927
// scanning install directories.
927-
int scanMode = SCAN_MONITOR | SCAN_NO_PATHS | SCAN_DEFER_DEX;
928+
int scanMode = SCAN_MONITOR | SCAN_NO_PATHS | SCAN_DEFER_DEX | SCAN_BOOTING;
928929
if (mNoDexOpt) {
929930
Slog.w(TAG, "Running ENG build: no pre-dexopt!");
930931
scanMode |= SCAN_NO_DEX;
@@ -3750,17 +3751,34 @@ private PackageParser.Package scanPackageLI(PackageParser.Package pkg,
37503751
} else {
37513752
// This is a normal package, need to make its data directory.
37523753
dataPath = getDataPathForPackage(pkg.packageName, 0);
3753-
3754+
37543755
boolean uidError = false;
3755-
3756+
37563757
if (dataPath.exists()) {
3758+
// XXX should really do this check for each user.
37573759
mOutPermissions[1] = 0;
37583760
FileUtils.getPermissions(dataPath.getPath(), mOutPermissions);
37593761

37603762
// If we have mismatched owners for the data path, we have a problem.
37613763
if (mOutPermissions[1] != pkg.applicationInfo.uid) {
37623764
boolean recovered = false;
3763-
if ((parseFlags&PackageParser.PARSE_IS_SYSTEM) != 0) {
3765+
if (mOutPermissions[1] == 0) {
3766+
// The directory somehow became owned by root. Wow.
3767+
// This is probably because the system was stopped while
3768+
// installd was in the middle of messing with its libs
3769+
// directory. Ask installd to fix that.
3770+
int ret = mInstaller.fixUid(pkgName, pkg.applicationInfo.uid,
3771+
pkg.applicationInfo.uid);
3772+
if (ret >= 0) {
3773+
recovered = true;
3774+
String msg = "Package " + pkg.packageName
3775+
+ " unexpectedly changed to uid 0; recovered to " +
3776+
+ pkg.applicationInfo.uid;
3777+
reportSettingsProblem(Log.WARN, msg);
3778+
}
3779+
}
3780+
if (!recovered && ((parseFlags&PackageParser.PARSE_IS_SYSTEM) != 0
3781+
|| (scanMode&SCAN_BOOTING) != 0)) {
37643782
// If this is a system app, we can at least delete its
37653783
// current data so the application will still work.
37663784
int ret = mInstaller.remove(pkgName, 0);
@@ -3769,7 +3787,9 @@ private PackageParser.Package scanPackageLI(PackageParser.Package pkg,
37693787
// Remove the data directories for all users
37703788
sUserManager.removePackageForAllUsers(pkgName);
37713789
// Old data gone!
3772-
String msg = "System package " + pkg.packageName
3790+
String prefix = (parseFlags&PackageParser.PARSE_IS_SYSTEM) != 0
3791+
? "System package " : "Third party package ";
3792+
String msg = prefix + pkg.packageName
37733793
+ " has changed from uid: "
37743794
+ mOutPermissions[1] + " to "
37753795
+ pkg.applicationInfo.uid + "; old data erased";
@@ -3781,7 +3801,7 @@ private PackageParser.Package scanPackageLI(PackageParser.Package pkg,
37813801
pkg.applicationInfo.uid);
37823802
if (ret == -1) {
37833803
// Ack should not happen!
3784-
msg = "System package " + pkg.packageName
3804+
msg = prefix + pkg.packageName
37853805
+ " could not have data directory re-created after delete.";
37863806
reportSettingsProblem(Log.WARN, msg);
37873807
mLastScanError = PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE;
@@ -3794,6 +3814,11 @@ private PackageParser.Package scanPackageLI(PackageParser.Package pkg,
37943814
if (!recovered) {
37953815
mHasSystemUidErrors = true;
37963816
}
3817+
} else if (!recovered) {
3818+
// If we allow this install to proceed, we will be broken.
3819+
// Abort, abort!
3820+
mLastScanError = PackageManager.INSTALL_FAILED_UID_CHANGED;
3821+
return null;
37973822
}
37983823
if (!recovered) {
37993824
pkg.applicationInfo.dataDir = "/mismatched_uid/settings_"

0 commit comments

Comments
 (0)