diff --git a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java index cb28254f70db6..bd2a97df1d7a6 100644 --- a/services/companion/java/com/android/server/companion/AssociationStoreImpl.java +++ b/services/companion/java/com/android/server/companion/AssociationStoreImpl.java @@ -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); diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index 13a5a28299458..02a0a95791233 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -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); } diff --git a/services/companion/java/com/android/server/companion/PersistentDataStore.java b/services/companion/java/com/android/server/companion/PersistentDataStore.java index d0cc12286b128..36393894f7277 100644 --- a/services/companion/java/com/android/server/companion/PersistentDataStore.java +++ b/services/companion/java/com/android/server/companion/PersistentDataStore.java @@ -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 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;