Merge changes from topic "presubmit-am-b8c92f3352a44b68aa16fbbc6837c783" into tm-mainline-prod
* changes:
[automerge] Guard against incorrect ID assignment in CdmService 2p: 8228658b69
Guard against incorrect ID assignment in CdmService
This commit is contained in:
committed by
Android (Google) Code Review
commit
ac1bd8a64a
@@ -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