From 4bfc7a5dbba1515975f202d7a545cf7fe42bc021 Mon Sep 17 00:00:00 2001 From: Amith Yamasani Date: Mon, 9 Jul 2018 12:07:35 -0700 Subject: Fix for incorrect cycle evaluation in computeOomAdj DO NOT MERGE Use the conservative value of adj and procstate if at least one evaluation pass was completed, even if the value is not final. The later iterations through the procs that have cycles will elevate the apps if necessary. Otherwise the dependencies will just get stuck in a low state. Bug: 79643956 Test: Manual test of connecting to AA and turning off screen atest CtsAppTestCases:ActivityManagerProcessStateTest Change-Id: If520eb239935782e2487b16e8bb650ded775f184 (cherry picked from commit d2aa4e1913c05d8c21e81eef3ad941565aded7d6) --- .../android/server/am/ActivityManagerService.java | 36 +++++++++++++++------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 692d6063fc02..19b25f0590a1 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -23039,6 +23039,7 @@ public class ActivityManagerService extends IActivityManager.Stub // The process is being computed, so there is a cycle. We cannot // rely on this process's state. app.containsCycle = true; + return false; } } @@ -23063,6 +23064,7 @@ public class ActivityManagerService extends IActivityManager.Stub final int logUid = mCurOomAdjUid; int prevAppAdj = app.curAdj; + int prevProcState = app.curProcState; if (app.maxAdj <= ProcessList.FOREGROUND_APP_ADJ) { // The max adjustment doesn't allow this app to be anything @@ -23541,11 +23543,16 @@ public class ActivityManagerService extends IActivityManager.Stub ProcessRecord client = cr.binding.client; computeOomAdjLocked(client, cachedAdj, TOP_APP, doingAll, now); if (client.containsCycle) { - // We've detected a cycle. We should ignore this connection and allow - // this process to retry computeOomAdjLocked later in case a later-checked - // connection from a client would raise its priority legitimately. + // We've detected a cycle. We should retry computeOomAdjLocked later in + // case a later-checked connection from a client would raise its + // priority legitimately. app.containsCycle = true; - continue; + // If the client has not been completely evaluated, skip using its + // priority. Else use the conservative value for now and look for a + // better state in the next iteration. + if (client.completedAdjSeq < mAdjSeq) { + continue; + } } int clientAdj = client.curRawAdj; int clientProcState = client.curProcState; @@ -23768,11 +23775,16 @@ public class ActivityManagerService extends IActivityManager.Stub } computeOomAdjLocked(client, cachedAdj, TOP_APP, doingAll, now); if (client.containsCycle) { - // We've detected a cycle. We should ignore this connection and allow - // this process to retry computeOomAdjLocked later in case a later-checked - // connection from a client would raise its priority legitimately. + // We've detected a cycle. We should retry computeOomAdjLocked later in + // case a later-checked connection from a client would raise its + // priority legitimately. app.containsCycle = true; - continue; + // If the client has not been completely evaluated, skip using its + // priority. Else use the conservative value for now and look for a + // better state in the next iteration. + if (client.completedAdjSeq < mAdjSeq) { + continue; + } } int clientAdj = client.curRawAdj; int clientProcState = client.curProcState; @@ -24004,8 +24016,8 @@ public class ActivityManagerService extends IActivityManager.Stub app.foregroundActivities = foregroundActivities; app.completedAdjSeq = mAdjSeq; - // if curAdj is less than prevAppAdj, then this process was promoted - return app.curAdj < prevAppAdj; + // if curAdj or curProcState improved, then this process was promoted + return app.curAdj < prevAppAdj || app.curProcState < prevProcState; } /** @@ -25058,7 +25070,7 @@ public class ActivityManagerService extends IActivityManager.Stub // - Continue retrying until no process was promoted. // - Iterate from least important to most important. int cycleCount = 0; - while (retryCycles) { + while (retryCycles && cycleCount < 10) { cycleCount++; retryCycles = false; @@ -25073,12 +25085,14 @@ public class ActivityManagerService extends IActivityManager.Stub for (int i=0; i=0; i--) { ProcessRecord app = mLruProcesses.get(i); if (!app.killedByAm && app.thread != null) { -- cgit v1.2.3 From 962fb40991f15be4f688d960aa00073683ebdd20 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 7 Aug 2018 15:02:17 -0600 Subject: DO NOT MERGE. Persistable Uri grants still require permissions. When FLAG_GRANT_PERSISTABLE_URI_PERMISSION is requested, we still need to check permissions between the source and target packages, instead of shortcutting past them. The spirit of the original change is remains intact: if the caller requested FLAG_GRANT_PERSISTABLE_URI_PERMISSION, then we avoid returning "-1", which would prevent the grant data structure from being allocated. Bug: 111934948 Test: atest android.appsecurity.cts.AppSecurityTests Change-Id: Ief0fc922aa09fc3d9bb6a126c2ff5855347cd030 Merged-In: Ief0fc922aa09fc3d9bb6a126c2ff5855347cd030 (cherry picked from commit d6a6e7127cc341ca875d9d13cf7a864d9f20b479) --- .../com/android/server/am/ActivityManagerService.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 19b25f0590a1..bbd2ca4cb394 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -9626,10 +9626,17 @@ public class ActivityManagerService extends IActivityManager.Stub } } - // If we're extending a persistable grant, then we always need to create - // the grant data structure so that take/release APIs work + // Figure out the value returned when access is allowed + final int allowedResult; if ((modeFlags & Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION) != 0) { - return targetUid; + // If we're extending a persistable grant, then we need to return + // "targetUid" so that we always create a grant data structure to + // support take/release APIs + allowedResult = targetUid; + } else { + // Otherwise, we can return "-1" to indicate that no grant data + // structures need to be created + allowedResult = -1; } if (targetUid >= 0) { @@ -9638,7 +9645,7 @@ public class ActivityManagerService extends IActivityManager.Stub // No need to grant the target this permission. if (DEBUG_URI_PERMISSION) Slog.v(TAG_URI_PERMISSION, "Target " + targetPkg + " already has full permission to " + grantUri); - return -1; + return allowedResult; } } else { // First... there is no target package, so can anyone access it? @@ -9673,7 +9680,7 @@ public class ActivityManagerService extends IActivityManager.Stub } } if (allowed) { - return -1; + return allowedResult; } } -- cgit v1.2.3 From 462aaeaa616e0bb1342e8ef7b472acc0cbc93deb Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 25 Jul 2018 14:01:59 -0600 Subject: DO NOT MERGE. Execute "strict" queries with extra parentheses. SQLiteQueryBuilder has a setStrict() mode which can be used to detect SQL attacks from untrusted sources, which it does by running each query twice: once with an extra set of parentheses, and if that succeeds, it runs the original query verbatim. This sadly doesn't catch inputs of the type "1=1) OR (1=1", which creates valid statements for both tests above, but the final executed query ends up leaking data due to SQLite operator precedence. Instead, we need to continue compiling both variants, but we need to execute the query with the additional parentheses to ensure data won't be leaked. Test: atest cts/tests/tests/database/src/android/database/sqlite/cts/SQLiteQueryBuilderTest.java Bug: 111085900 Change-Id: I6e8746fa48f9de13adae37d2990de11c9c585381 Merged-In: I6e8746fa48f9de13adae37d2990de11c9c585381 (cherry picked from commit 57b04a86802ff879af78e782a8582462323e34e7) --- .../database/sqlite/SQLiteQueryBuilder.java | 25 ++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/core/java/android/database/sqlite/SQLiteQueryBuilder.java b/core/java/android/database/sqlite/SQLiteQueryBuilder.java index c6c676f81758..f3efc6fd98d5 100644 --- a/core/java/android/database/sqlite/SQLiteQueryBuilder.java +++ b/core/java/android/database/sqlite/SQLiteQueryBuilder.java @@ -376,6 +376,11 @@ public class SQLiteQueryBuilder return null; } + final String sql; + final String unwrappedSql = buildQuery( + projectionIn, selection, groupBy, having, + sortOrder, limit); + if (mStrict && selection != null && selection.length() > 0) { // Validate the user-supplied selection to detect syntactic anomalies // in the selection string that could indicate a SQL injection attempt. @@ -384,15 +389,23 @@ public class SQLiteQueryBuilder // originally specified. An attacker cannot create an expression that // would escape the SQL expression while maintaining balanced parentheses // in both the wrapped and original forms. - String sqlForValidation = buildQuery(projectionIn, "(" + selection + ")", groupBy, + + // NOTE: The ordering of the below operations is important; we must + // execute the wrapped query to ensure the untrusted clause has been + // fully isolated. + + // Validate the unwrapped query + db.validateSql(unwrappedSql, cancellationSignal); // will throw if query is invalid + + // Execute wrapped query for extra protection + final String wrappedSql = buildQuery(projectionIn, "(" + selection + ")", groupBy, having, sortOrder, limit); - db.validateSql(sqlForValidation, cancellationSignal); // will throw if query is invalid + sql = wrappedSql; + } else { + // Execute unwrapped query + sql = unwrappedSql; } - String sql = buildQuery( - projectionIn, selection, groupBy, having, - sortOrder, limit); - if (Log.isLoggable(TAG, Log.DEBUG)) { Log.d(TAG, "Performing query: " + sql); } -- cgit v1.2.3 From ebc250d16c747f4161167b5ff58b3aea88b37acf Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 25 Jul 2018 14:52:14 -0600 Subject: DO NOT MERGE. Extend SQLiteQueryBuilder for update and delete. Developers often accept selection clauses from untrusted code, and SQLiteQueryBuilder already supports a "strict" mode to help catch SQL injection attacks. This change extends the builder to support update() and delete() calls, so that we can help secure those selection clauses too. Bug: 111085900 Test: atest packages/providers/DownloadProvider/tests/ Test: atest cts/tests/app/src/android/app/cts/DownloadManagerTest.java Test: atest cts/tests/tests/database/src/android/database/sqlite/cts/SQLiteQueryBuilderTest.java Change-Id: Ib4fc8400f184755ee7e971ab5f2095186341730c Merged-In: Ib4fc8400f184755ee7e971ab5f2095186341730c (cherry picked from commit 506994268bc4fa07d8798b7737a2952f74b8fd04) --- .../android/database/sqlite/SQLiteDatabase.java | 3 +- .../database/sqlite/SQLiteQueryBuilder.java | 245 ++++++++++++++++++--- 2 files changed, 218 insertions(+), 30 deletions(-) diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java index 6adae25fc38d..96cd043b8676 100644 --- a/core/java/android/database/sqlite/SQLiteDatabase.java +++ b/core/java/android/database/sqlite/SQLiteDatabase.java @@ -1748,7 +1748,8 @@ public final class SQLiteDatabase extends SQLiteClosable { executeSql(sql, bindArgs); } - private int executeSql(String sql, Object[] bindArgs) throws SQLException { + /** {@hide} */ + public int executeSql(String sql, Object[] bindArgs) throws SQLException { acquireReference(); try { final int statementType = DatabaseUtils.getSqlStatementType(sql); diff --git a/core/java/android/database/sqlite/SQLiteQueryBuilder.java b/core/java/android/database/sqlite/SQLiteQueryBuilder.java index f3efc6fd98d5..442e4cf0110c 100644 --- a/core/java/android/database/sqlite/SQLiteQueryBuilder.java +++ b/core/java/android/database/sqlite/SQLiteQueryBuilder.java @@ -16,17 +16,25 @@ package android.database.sqlite; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.content.ContentValues; import android.database.Cursor; import android.database.DatabaseUtils; +import android.os.Build; import android.os.CancellationSignal; import android.os.OperationCanceledException; import android.provider.BaseColumns; import android.text.TextUtils; import android.util.Log; +import libcore.util.EmptyArray; + +import java.util.Arrays; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; @@ -95,9 +103,6 @@ public class SQLiteQueryBuilder if (mWhereClause == null) { mWhereClause = new StringBuilder(inWhere.length() + 16); } - if (mWhereClause.length() == 0) { - mWhereClause.append('('); - } mWhereClause.append(inWhere); } @@ -115,9 +120,6 @@ public class SQLiteQueryBuilder if (mWhereClause == null) { mWhereClause = new StringBuilder(inWhere.length() + 16); } - if (mWhereClause.length() == 0) { - mWhereClause.append('('); - } DatabaseUtils.appendEscapedSQLString(mWhereClause, inWhere); } @@ -398,7 +400,7 @@ public class SQLiteQueryBuilder db.validateSql(unwrappedSql, cancellationSignal); // will throw if query is invalid // Execute wrapped query for extra protection - final String wrappedSql = buildQuery(projectionIn, "(" + selection + ")", groupBy, + final String wrappedSql = buildQuery(projectionIn, wrap(selection), groupBy, having, sortOrder, limit); sql = wrappedSql; } else { @@ -406,15 +408,149 @@ public class SQLiteQueryBuilder sql = unwrappedSql; } + final String[] sqlArgs = selectionArgs; if (Log.isLoggable(TAG, Log.DEBUG)) { - Log.d(TAG, "Performing query: " + sql); + if (Build.IS_DEBUGGABLE) { + Log.d(TAG, sql + " with args " + Arrays.toString(sqlArgs)); + } else { + Log.d(TAG, sql); + } } return db.rawQueryWithFactory( - mFactory, sql, selectionArgs, + mFactory, sql, sqlArgs, SQLiteDatabase.findEditTable(mTables), cancellationSignal); // will throw if query is invalid } + /** + * Perform an update by combining all current settings and the + * information passed into this method. + * + * @param db the database to update on + * @param selection A filter declaring which rows to return, + * formatted as an SQL WHERE clause (excluding the WHERE + * itself). Passing null will return all rows for the given URL. + * @param selectionArgs You may include ?s in selection, which + * will be replaced by the values from selectionArgs, in order + * that they appear in the selection. The values will be bound + * as Strings. + * @return the number of rows updated + * @hide + */ + public int update(@NonNull SQLiteDatabase db, @NonNull ContentValues values, + @Nullable String selection, @Nullable String[] selectionArgs) { + Objects.requireNonNull(mTables, "No tables defined"); + Objects.requireNonNull(db, "No database defined"); + Objects.requireNonNull(values, "No values defined"); + + final String sql; + final String unwrappedSql = buildUpdate(values, selection); + + if (mStrict) { + // Validate the user-supplied selection to detect syntactic anomalies + // in the selection string that could indicate a SQL injection attempt. + // The idea is to ensure that the selection clause is a valid SQL expression + // by compiling it twice: once wrapped in parentheses and once as + // originally specified. An attacker cannot create an expression that + // would escape the SQL expression while maintaining balanced parentheses + // in both the wrapped and original forms. + + // NOTE: The ordering of the below operations is important; we must + // execute the wrapped query to ensure the untrusted clause has been + // fully isolated. + + // Validate the unwrapped query + db.validateSql(unwrappedSql, null); // will throw if query is invalid + + // Execute wrapped query for extra protection + final String wrappedSql = buildUpdate(values, wrap(selection)); + sql = wrappedSql; + } else { + // Execute unwrapped query + sql = unwrappedSql; + } + + if (selectionArgs == null) { + selectionArgs = EmptyArray.STRING; + } + final String[] rawKeys = values.keySet().toArray(EmptyArray.STRING); + final int valuesLength = rawKeys.length; + final Object[] sqlArgs = new Object[valuesLength + selectionArgs.length]; + for (int i = 0; i < sqlArgs.length; i++) { + if (i < valuesLength) { + sqlArgs[i] = values.get(rawKeys[i]); + } else { + sqlArgs[i] = selectionArgs[i - valuesLength]; + } + } + if (Log.isLoggable(TAG, Log.DEBUG)) { + if (Build.IS_DEBUGGABLE) { + Log.d(TAG, sql + " with args " + Arrays.toString(sqlArgs)); + } else { + Log.d(TAG, sql); + } + } + return db.executeSql(sql, sqlArgs); + } + + /** + * Perform a delete by combining all current settings and the + * information passed into this method. + * + * @param db the database to delete on + * @param selection A filter declaring which rows to return, + * formatted as an SQL WHERE clause (excluding the WHERE + * itself). Passing null will return all rows for the given URL. + * @param selectionArgs You may include ?s in selection, which + * will be replaced by the values from selectionArgs, in order + * that they appear in the selection. The values will be bound + * as Strings. + * @return the number of rows deleted + * @hide + */ + public int delete(@NonNull SQLiteDatabase db, @Nullable String selection, + @Nullable String[] selectionArgs) { + Objects.requireNonNull(mTables, "No tables defined"); + Objects.requireNonNull(db, "No database defined"); + + final String sql; + final String unwrappedSql = buildDelete(selection); + + if (mStrict) { + // Validate the user-supplied selection to detect syntactic anomalies + // in the selection string that could indicate a SQL injection attempt. + // The idea is to ensure that the selection clause is a valid SQL expression + // by compiling it twice: once wrapped in parentheses and once as + // originally specified. An attacker cannot create an expression that + // would escape the SQL expression while maintaining balanced parentheses + // in both the wrapped and original forms. + + // NOTE: The ordering of the below operations is important; we must + // execute the wrapped query to ensure the untrusted clause has been + // fully isolated. + + // Validate the unwrapped query + db.validateSql(unwrappedSql, null); // will throw if query is invalid + + // Execute wrapped query for extra protection + final String wrappedSql = buildDelete(wrap(selection)); + sql = wrappedSql; + } else { + // Execute unwrapped query + sql = unwrappedSql; + } + + final String[] sqlArgs = selectionArgs; + if (Log.isLoggable(TAG, Log.DEBUG)) { + if (Build.IS_DEBUGGABLE) { + Log.d(TAG, sql + " with args " + Arrays.toString(sqlArgs)); + } else { + Log.d(TAG, sql); + } + } + return db.executeSql(sql, sqlArgs); + } + /** * Construct a SELECT statement suitable for use in a group of * SELECT statements that will be joined through UNION operators @@ -447,28 +583,10 @@ public class SQLiteQueryBuilder String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit) { String[] projection = computeProjection(projectionIn); - - StringBuilder where = new StringBuilder(); - boolean hasBaseWhereClause = mWhereClause != null && mWhereClause.length() > 0; - - if (hasBaseWhereClause) { - where.append(mWhereClause.toString()); - where.append(')'); - } - - // Tack on the user's selection, if present. - if (selection != null && selection.length() > 0) { - if (hasBaseWhereClause) { - where.append(" AND "); - } - - where.append('('); - where.append(selection); - where.append(')'); - } + String where = computeWhere(selection); return buildQueryString( - mDistinct, mTables, projection, where.toString(), + mDistinct, mTables, projection, where, groupBy, having, sortOrder, limit); } @@ -485,6 +603,42 @@ public class SQLiteQueryBuilder return buildQuery(projectionIn, selection, groupBy, having, sortOrder, limit); } + /** {@hide} */ + public String buildUpdate(ContentValues values, String selection) { + if (values == null || values.size() == 0) { + throw new IllegalArgumentException("Empty values"); + } + + StringBuilder sql = new StringBuilder(120); + sql.append("UPDATE "); + sql.append(mTables); + sql.append(" SET "); + + final String[] rawKeys = values.keySet().toArray(EmptyArray.STRING); + for (int i = 0; i < rawKeys.length; i++) { + if (i > 0) { + sql.append(','); + } + sql.append(rawKeys[i]); + sql.append("=?"); + } + + final String where = computeWhere(selection); + appendClause(sql, " WHERE ", where); + return sql.toString(); + } + + /** {@hide} */ + public String buildDelete(String selection) { + StringBuilder sql = new StringBuilder(120); + sql.append("DELETE FROM "); + sql.append(mTables); + + final String where = computeWhere(selection); + appendClause(sql, " WHERE ", where); + return sql.toString(); + } + /** * Construct a SELECT statement suitable for use in a group of * SELECT statements that will be joined through UNION operators @@ -658,4 +812,37 @@ public class SQLiteQueryBuilder } return null; } + + private @Nullable String computeWhere(@Nullable String selection) { + final boolean hasInternal = !TextUtils.isEmpty(mWhereClause); + final boolean hasExternal = !TextUtils.isEmpty(selection); + + if (hasInternal || hasExternal) { + final StringBuilder where = new StringBuilder(); + if (hasInternal) { + where.append('(').append(mWhereClause).append(')'); + } + if (hasInternal && hasExternal) { + where.append(" AND "); + } + if (hasExternal) { + where.append('(').append(selection).append(')'); + } + return where.toString(); + } else { + return null; + } + } + + /** + * Wrap given argument in parenthesis, unless it's {@code null} or + * {@code ()}, in which case return it verbatim. + */ + private @Nullable String wrap(@Nullable String arg) { + if (TextUtils.isEmpty(arg)) { + return arg; + } else { + return "(" + arg + ")"; + } + } } -- cgit v1.2.3 From dbe404dc74c77bfa6c592645db1870d6023d9d3b Mon Sep 17 00:00:00 2001 From: Hongming Jin Date: Tue, 19 Jun 2018 12:35:37 -0700 Subject: RESTRICT AUTOMERGE: Revoke permissions defined in a to-be removed package. Bug: 67319274 Test: run cts-dev --module CtsPermissionTestCases --test android.permission.cts.RemovePermissionTest#permissionShouldBeRevokedIfRemoved Change-Id: Id23535d6c4d2fcf5c86dc1338af13a561b32154c (cherry picked from commit fde947837c848b51a830b74b28f96f9808405492) --- .../android/server/pm/PackageManagerService.java | 3 +- .../pm/permission/PermissionManagerInternal.java | 8 ++- .../pm/permission/PermissionManagerService.java | 81 +++++++++++++++++++--- 3 files changed, 79 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 9ed2b9c18546..6286a242dba2 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -12344,7 +12344,8 @@ public class PackageManagerService extends IPackageManager.Stub if (DEBUG_REMOVE) Log.d(TAG, " Activities: " + r); } - mPermissionManager.removeAllPermissions(pkg, chatty); + final ArrayList allPackageNames = new ArrayList<>(mPackages.keySet()); + mPermissionManager.removeAllPermissions(pkg, allPackageNames, mPermissionCallback, chatty); N = pkg.instrumentation.size(); r = null; diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java index a042fedf8b47..c3f23a81518a 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java @@ -115,7 +115,11 @@ public abstract class PermissionManagerInternal { */ public abstract void addAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty); public abstract void addAllPermissionGroups(@NonNull PackageParser.Package pkg, boolean chatty); - public abstract void removeAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty); + public abstract void removeAllPermissions( + @NonNull PackageParser.Package pkg, + @NonNull List allPackageNames, + @Nullable PermissionCallback permissionCallback, + boolean chatty); public abstract boolean addDynamicPermission(@NonNull PermissionInfo info, boolean async, int callingUid, @Nullable PermissionCallback callback); public abstract void removeDynamicPermission(@NonNull String permName, int callingUid, @@ -189,4 +193,4 @@ public abstract class PermissionManagerInternal { /** HACK HACK methods to allow for partial migration of data to the PermissionManager class */ public abstract @Nullable BasePermission getPermissionTEMP(@NonNull String permName); -} \ No newline at end of file +} diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index c51a72406b53..02c9049f008e 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -30,6 +30,7 @@ import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; +import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManagerInternal; import android.content.pm.PackageParser; @@ -37,6 +38,7 @@ import android.content.pm.PermissionGroupInfo; import android.content.pm.PermissionInfo; import android.content.pm.PackageParser.Package; import android.metrics.LogMaker; +import android.os.AsyncTask; import android.os.Binder; import android.os.Build; import android.os.Handler; @@ -455,8 +457,9 @@ public class PermissionManagerService { " to " + newPermissionGroupName); try { - revokeRuntimePermission(permissionName, packageName, false, - Process.SYSTEM_UID, userId, permissionCallback); + revokeRuntimePermission(permissionName, packageName, + mSettings.getPermission(permissionName), false, + Process.SYSTEM_UID, userId, permissionCallback, false); } catch (IllegalArgumentException e) { Slog.e(TAG, "Could not revoke " + permissionName + " from " + packageName, e); @@ -549,9 +552,59 @@ public class PermissionManagerService { } - private void removeAllPermissions(PackageParser.Package pkg, boolean chatty) { + private void revokeAllPermissions( + @NonNull List bps, + @NonNull List allPackageNames, + @Nullable PermissionCallback permissionCallback) { + AsyncTask.execute(() -> { + final int numRemovedPermissions = bps.size(); + for (int permissionNum = 0; permissionNum < numRemovedPermissions; permissionNum++) { + final int[] userIds = mUserManagerInt.getUserIds(); + final int numUserIds = userIds.length; + + final int numPackages = allPackageNames.size(); + for (int packageNum = 0; packageNum < numPackages; packageNum++) { + final String packageName = allPackageNames.get(packageNum); + final ApplicationInfo applicationInfo = mPackageManagerInt.getApplicationInfo( + packageName, 0, Process.SYSTEM_UID, UserHandle.USER_SYSTEM); + if (applicationInfo != null + && applicationInfo.targetSdkVersion < Build.VERSION_CODES.M) { + continue; + } + for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) { + final int userId = userIds[userIdNum]; + final String permissionName = bps.get(permissionNum).getName(); + if (checkPermission(permissionName, packageName, UserHandle.USER_SYSTEM, + userId) == PackageManager.PERMISSION_GRANTED) { + try { + revokeRuntimePermission( + permissionName, + packageName, + bps.get(permissionNum), + false, + Process.SYSTEM_UID, + userId, + permissionCallback, + true); + } catch (IllegalArgumentException e) { + Slog.e(TAG, "Could not revoke " + permissionName + " from " + + packageName, e); + } + } + } + } + } + }); + } + + private void removeAllPermissions( + @NonNull PackageParser.Package pkg, + @NonNull List allPackageNames, + @Nullable PermissionCallback permissionCallback, + boolean chatty) { synchronized (mLock) { int N = pkg.permissions.size(); + List bps = new ArrayList(N); StringBuilder r = null; for (int i=0; i allPackageNames, + PermissionCallback permissionCallback, boolean chatty) { + PermissionManagerService.this.removeAllPermissions( + pkg, allPackageNames, permissionCallback, chatty); } @Override public boolean addDynamicPermission(PermissionInfo info, boolean async, int callingUid, @@ -2110,7 +2170,8 @@ public class PermissionManagerService { boolean overridePolicy, int callingUid, int userId, PermissionCallback callback) { PermissionManagerService.this.revokeRuntimePermission(permName, packageName, - overridePolicy, callingUid, userId, callback); + mSettings.getPermission(permName), overridePolicy, callingUid, userId, + callback, false); } @Override public void updatePermissions(String packageName, Package pkg, boolean replaceGrant, -- cgit v1.2.3 From 54f661b16b308cf38d1b9703214591c0f83df64d Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Thu, 19 Jul 2018 16:22:02 -0700 Subject: Fix crash during cursor moving on BiDi text The crash was introduced by Ib66ef392c19c937718e7101f6d48fac3abe51ad0 The root cause of the crashing is requesting out-of-line access for the horizontal width. This invalid access is silently ignored by TextLine#measure() method but new implementation end up with out of bounds access. To makes behavior as old implementation, calling getHorizontal instead of accessing measured result array. Bug: 78464361, 111580019 Test: Manually done Change-Id: I5c5778718f6b397adbb1e4f2cf95e9f635f6e5c8 (cherry picked from commit 960647d582911ae7ab8b9491097898e6c313aaf1) Merged-In: I5c5778718f6b397adbb1e4f2cf95e9f635f6e5c8 (cherry picked from commit a1076fdaa54ebf56bb32bea43fb278f7470ff307) --- core/java/android/text/Layout.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/java/android/text/Layout.java b/core/java/android/text/Layout.java index 09af85db2620..c6f73cb47576 100644 --- a/core/java/android/text/Layout.java +++ b/core/java/android/text/Layout.java @@ -1583,7 +1583,8 @@ public abstract class Layout { } float get(final int offset) { - if (mHorizontals == null) { + if (mHorizontals == null || offset < mLineStartOffset + || offset >= mLineStartOffset + mHorizontals.length) { return getHorizontal(offset, mPrimary); } else { return mHorizontals[offset - mLineStartOffset]; -- cgit v1.2.3 From fc668f702435b0f70c53c67e53aea25a66fff96d Mon Sep 17 00:00:00 2001 From: Elisa Pascual Trevino Date: Wed, 29 Aug 2018 23:19:18 +0000 Subject: Revert "RESTRICT AUTOMERGE: Revoke permissions defined in a to-be removed package." This reverts commit fde947837c848b51a830b74b28f96f9808405492. Reason for revert: b/111752150 Change-Id: Ifc60261cd78e606e01f7a68626a1710699ff7bc7 (cherry picked from commit 307c6c50a7b374b4ab034560bb67a204071b5bca) --- .../android/server/pm/PackageManagerService.java | 3 +- .../pm/permission/PermissionManagerInternal.java | 8 +-- .../pm/permission/PermissionManagerService.java | 81 +++------------------- 3 files changed, 13 insertions(+), 79 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 6286a242dba2..9ed2b9c18546 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -12344,8 +12344,7 @@ public class PackageManagerService extends IPackageManager.Stub if (DEBUG_REMOVE) Log.d(TAG, " Activities: " + r); } - final ArrayList allPackageNames = new ArrayList<>(mPackages.keySet()); - mPermissionManager.removeAllPermissions(pkg, allPackageNames, mPermissionCallback, chatty); + mPermissionManager.removeAllPermissions(pkg, chatty); N = pkg.instrumentation.size(); r = null; diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java index c3f23a81518a..a042fedf8b47 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java @@ -115,11 +115,7 @@ public abstract class PermissionManagerInternal { */ public abstract void addAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty); public abstract void addAllPermissionGroups(@NonNull PackageParser.Package pkg, boolean chatty); - public abstract void removeAllPermissions( - @NonNull PackageParser.Package pkg, - @NonNull List allPackageNames, - @Nullable PermissionCallback permissionCallback, - boolean chatty); + public abstract void removeAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty); public abstract boolean addDynamicPermission(@NonNull PermissionInfo info, boolean async, int callingUid, @Nullable PermissionCallback callback); public abstract void removeDynamicPermission(@NonNull String permName, int callingUid, @@ -193,4 +189,4 @@ public abstract class PermissionManagerInternal { /** HACK HACK methods to allow for partial migration of data to the PermissionManager class */ public abstract @Nullable BasePermission getPermissionTEMP(@NonNull String permName); -} +} \ No newline at end of file diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index 02c9049f008e..c51a72406b53 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -30,7 +30,6 @@ import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; -import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManagerInternal; import android.content.pm.PackageParser; @@ -38,7 +37,6 @@ import android.content.pm.PermissionGroupInfo; import android.content.pm.PermissionInfo; import android.content.pm.PackageParser.Package; import android.metrics.LogMaker; -import android.os.AsyncTask; import android.os.Binder; import android.os.Build; import android.os.Handler; @@ -457,9 +455,8 @@ public class PermissionManagerService { " to " + newPermissionGroupName); try { - revokeRuntimePermission(permissionName, packageName, - mSettings.getPermission(permissionName), false, - Process.SYSTEM_UID, userId, permissionCallback, false); + revokeRuntimePermission(permissionName, packageName, false, + Process.SYSTEM_UID, userId, permissionCallback); } catch (IllegalArgumentException e) { Slog.e(TAG, "Could not revoke " + permissionName + " from " + packageName, e); @@ -552,59 +549,9 @@ public class PermissionManagerService { } - private void revokeAllPermissions( - @NonNull List bps, - @NonNull List allPackageNames, - @Nullable PermissionCallback permissionCallback) { - AsyncTask.execute(() -> { - final int numRemovedPermissions = bps.size(); - for (int permissionNum = 0; permissionNum < numRemovedPermissions; permissionNum++) { - final int[] userIds = mUserManagerInt.getUserIds(); - final int numUserIds = userIds.length; - - final int numPackages = allPackageNames.size(); - for (int packageNum = 0; packageNum < numPackages; packageNum++) { - final String packageName = allPackageNames.get(packageNum); - final ApplicationInfo applicationInfo = mPackageManagerInt.getApplicationInfo( - packageName, 0, Process.SYSTEM_UID, UserHandle.USER_SYSTEM); - if (applicationInfo != null - && applicationInfo.targetSdkVersion < Build.VERSION_CODES.M) { - continue; - } - for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) { - final int userId = userIds[userIdNum]; - final String permissionName = bps.get(permissionNum).getName(); - if (checkPermission(permissionName, packageName, UserHandle.USER_SYSTEM, - userId) == PackageManager.PERMISSION_GRANTED) { - try { - revokeRuntimePermission( - permissionName, - packageName, - bps.get(permissionNum), - false, - Process.SYSTEM_UID, - userId, - permissionCallback, - true); - } catch (IllegalArgumentException e) { - Slog.e(TAG, "Could not revoke " + permissionName + " from " - + packageName, e); - } - } - } - } - } - }); - } - - private void removeAllPermissions( - @NonNull PackageParser.Package pkg, - @NonNull List allPackageNames, - @Nullable PermissionCallback permissionCallback, - boolean chatty) { + private void removeAllPermissions(PackageParser.Package pkg, boolean chatty) { synchronized (mLock) { int N = pkg.permissions.size(); - List bps = new ArrayList(N); StringBuilder r = null; for (int i=0; i allPackageNames, - PermissionCallback permissionCallback, boolean chatty) { - PermissionManagerService.this.removeAllPermissions( - pkg, allPackageNames, permissionCallback, chatty); + public void removeAllPermissions(Package pkg, boolean chatty) { + PermissionManagerService.this.removeAllPermissions(pkg, chatty); } @Override public boolean addDynamicPermission(PermissionInfo info, boolean async, int callingUid, @@ -2170,8 +2110,7 @@ public class PermissionManagerService { boolean overridePolicy, int callingUid, int userId, PermissionCallback callback) { PermissionManagerService.this.revokeRuntimePermission(permName, packageName, - mSettings.getPermission(permName), overridePolicy, callingUid, userId, - callback, false); + overridePolicy, callingUid, userId, callback); } @Override public void updatePermissions(String packageName, Package pkg, boolean replaceGrant, -- cgit v1.2.3