Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0b6c9d31 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Automerger Merge Worker
Browse files

Merge "Guard against incorrect ID assignment in CdmService" into tm-dev am: eaceca75 

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/17654874



Change-Id: I746cf4fc2a284274d7914b713ce929529ddfd582
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 5a3fccc4 eaceca75

services/companion/java/com/android/server/companion/AssociationStoreImpl.java

+2 −1
Original line number Diff line number Diff line
@@ -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);

services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java

+7 −1
Original line number Diff line number Diff line
@@ -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);

            }

services/companion/java/com/android/server/companion/PersistentDataStore.java

+22 −2
Original line number Diff line number Diff line
@@ -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;