Guard against incorrect ID assignment in CdmService
Do not assume that all previously assigned association IDs were assigned correctly in regard to ranges of associations ID allocated to each user when generating an association ID for a new association in CompanionDeviceManagerService.getNewAssociationIdForPackage(). Also Slog.e() when detect such errors in existing Associations as well as when detect ID conflicts in AssociationStoreImpl.addAssociation(). Bug: 224736262 Bug: 223995714 Test: atest CtsCompanionDeviceManagerCoreTestCases Change-Id: Ibe14cfda9718b3e473f0f157bbfef2f7051819c2
This commit is contained in:
@@ -23,6 +23,7 @@ import android.annotation.UserIdInt;
|
||||
import android.companion.AssociationInfo;
|
||||
import android.net.MacAddress;
|
||||
import android.util.Log;
|
||||
import android.util.Slog;
|
||||
import android.util.SparseArray;
|
||||
|
||||
import com.android.internal.annotations.GuardedBy;
|
||||
@@ -80,7 +81,7 @@ class AssociationStoreImpl implements AssociationStore {
|
||||
|
||||
synchronized (mLock) {
|
||||
if (mIdMap.containsKey(id)) {
|
||||
if (DEBUG) Log.w(TAG, "Association already stored.");
|
||||
Slog.e(TAG, "Association with id " + id + " already exists.");
|
||||
return;
|
||||
}
|
||||
mIdMap.put(id, association);
|
||||
|
||||
@@ -813,7 +813,13 @@ public class CompanionDeviceManagerService extends SystemService {
|
||||
synchronized (mPreviouslyUsedIds) {
|
||||
// First: collect all IDs currently in use for this user's Associations.
|
||||
final SparseBooleanArray usedIds = new SparseBooleanArray();
|
||||
for (AssociationInfo it : mAssociationStore.getAssociationsForUser(userId)) {
|
||||
|
||||
// We should really only be checking associations for the given user (i.e.:
|
||||
// mAssociationStore.getAssociationsForUser(userId)), BUT in the past we've got in a
|
||||
// state where association IDs were not assigned correctly in regard to
|
||||
// user-to-association-ids-range (e.g. associations with IDs from 1 to 100,000 should
|
||||
// always belong to u0), so let's check all the associations.
|
||||
for (AssociationInfo it : mAssociationStore.getAssociations()) {
|
||||
usedIds.put(it.getId(), true);
|
||||
}
|
||||
|
||||
|
||||
@@ -25,6 +25,8 @@ import static com.android.internal.util.XmlUtils.writeBooleanAttribute;
|
||||
import static com.android.internal.util.XmlUtils.writeIntAttribute;
|
||||
import static com.android.internal.util.XmlUtils.writeLongAttribute;
|
||||
import static com.android.internal.util.XmlUtils.writeStringAttribute;
|
||||
import static com.android.server.companion.CompanionDeviceManagerService.getFirstAssociationIdForUser;
|
||||
import static com.android.server.companion.CompanionDeviceManagerService.getLastAssociationIdForUser;
|
||||
import static com.android.server.companion.DataStoreUtils.createStorageFileForUser;
|
||||
import static com.android.server.companion.DataStoreUtils.isEndOfTag;
|
||||
import static com.android.server.companion.DataStoreUtils.isStartOfTag;
|
||||
@@ -194,7 +196,25 @@ final class PersistentDataStore {
|
||||
|
||||
// Associations for all users are stored in a single "flat" set: so we read directly
|
||||
// into it.
|
||||
readStateForUser(userId, allAssociationsOut, previouslyUsedIds);
|
||||
final Set<AssociationInfo> associationsForUser = new HashSet<>();
|
||||
readStateForUser(userId, associationsForUser, previouslyUsedIds);
|
||||
|
||||
// Go through all the associations for the user and check if their IDs are within
|
||||
// the allowed range (for the user).
|
||||
final int firstAllowedId = getFirstAssociationIdForUser(userId);
|
||||
final int lastAllowedId = getLastAssociationIdForUser(userId);
|
||||
for (AssociationInfo association : associationsForUser) {
|
||||
final int id = association.getId();
|
||||
if (id < firstAllowedId || id > lastAllowedId) {
|
||||
Slog.e(TAG, "Wrong association ID assignment: " + id + ". "
|
||||
+ "Association belongs to u" + userId + " and thus its ID should be "
|
||||
+ "within [" + firstAllowedId + ", " + lastAllowedId + "] range.");
|
||||
// TODO(b/224736262): try fixing (re-assigning) the ID?
|
||||
}
|
||||
}
|
||||
|
||||
// Add user's association to the "output" set.
|
||||
allAssociationsOut.addAll(associationsForUser);
|
||||
|
||||
// Save previously used IDs for this user into the "out" structure.
|
||||
previouslyUsedIdsPerUserOut.append(userId, previouslyUsedIds);
|
||||
@@ -369,7 +389,7 @@ final class PersistentDataStore {
|
||||
// existing ones from the backup files. And the fact that we are reading from a V0 file,
|
||||
// means that CDM hasn't assigned any IDs yet, so we can just start from the first available
|
||||
// id for each user (eg. 1 for user 0; 100 001 - for user 1; 200 001 - for user 2; etc).
|
||||
int associationId = CompanionDeviceManagerService.getFirstAssociationIdForUser(userId);
|
||||
int associationId = getFirstAssociationIdForUser(userId);
|
||||
while (true) {
|
||||
parser.nextTag();
|
||||
if (isEndOfTag(parser, XML_TAG_ASSOCIATIONS)) break;
|
||||
|
||||
Reference in New Issue
Block a user