Follow-up fixes to MusicRecognitionManagerService

Added a check to only allow the default MusicRecognitionService to be called, unless the caller is the same app that hosts the MusicRecognitionService, as is the case in tests.

Refactored MusicRecognitionManagerPerUserService to not hold the client's callback as a field to prevent leaks and needing to null it out (then requiring null checks).  Instead, it's always a local variable or a final field in ephemeral objects.

Bug: 174744852
Bug: 168696277
Test: atest CtsMusicRecognitionTestCases
Change-Id: I70ff3c38da7096c60f82fe75b47674b4b233e672
This commit is contained in:
Nick Moukhine
2020-12-22 16:26:51 +01:00
parent a0334be887
commit 5b7c195b0d
3 changed files with 94 additions and 21 deletions

View File

@@ -16,6 +16,7 @@
package com.android.server.musicrecognition;
import static android.media.musicrecognition.MusicRecognitionManager.RECOGNITION_FAILED_AUDIO_UNAVAILABLE;
import static android.media.musicrecognition.MusicRecognitionManager.RECOGNITION_FAILED_SERVICE_KILLED;
import static android.media.musicrecognition.MusicRecognitionManager.RECOGNITION_FAILED_SERVICE_UNAVAILABLE;
import static android.media.musicrecognition.MusicRecognitionManager.RecognitionFailureCode;
@@ -64,10 +65,6 @@ public final class MusicRecognitionManagerPerUserService extends
@GuardedBy("mLock")
private RemoteMusicRecognitionService mRemoteService;
private MusicRecognitionServiceCallback mRemoteServiceCallback =
new MusicRecognitionServiceCallback();
private IMusicRecognitionManagerCallback mCallback;
MusicRecognitionManagerPerUserService(
@NonNull MusicRecognitionManagerService primary,
@NonNull Object lock, int userId) {
@@ -100,7 +97,8 @@ public final class MusicRecognitionManagerPerUserService extends
@GuardedBy("mLock")
@Nullable
private RemoteMusicRecognitionService ensureRemoteServiceLocked() {
private RemoteMusicRecognitionService ensureRemoteServiceLocked(
IMusicRecognitionManagerCallback clientCallback) {
if (mRemoteService == null) {
final String serviceName = getComponentNameLocked();
if (serviceName == null) {
@@ -113,7 +111,8 @@ public final class MusicRecognitionManagerPerUserService extends
mRemoteService = new RemoteMusicRecognitionService(getContext(),
serviceComponent, mUserId, this,
mRemoteServiceCallback, mMaster.isBindInstantServiceAllowed(),
new MusicRecognitionServiceCallback(clientCallback),
mMaster.isBindInstantServiceAllowed(),
mMaster.verbose);
}
@@ -130,13 +129,14 @@ public final class MusicRecognitionManagerPerUserService extends
@NonNull IBinder callback) {
int maxAudioLengthSeconds = Math.min(recognitionRequest.getMaxAudioLengthSeconds(),
MAX_STREAMING_SECONDS);
mCallback = IMusicRecognitionManagerCallback.Stub.asInterface(callback);
IMusicRecognitionManagerCallback clientCallback =
IMusicRecognitionManagerCallback.Stub.asInterface(callback);
AudioRecord audioRecord = createAudioRecord(recognitionRequest, maxAudioLengthSeconds);
mRemoteService = ensureRemoteServiceLocked();
mRemoteService = ensureRemoteServiceLocked(clientCallback);
if (mRemoteService == null) {
try {
mCallback.onRecognitionFailed(
clientCallback.onRecognitionFailed(
RECOGNITION_FAILED_SERVICE_UNAVAILABLE);
} catch (RemoteException e) {
// Ignored.
@@ -147,7 +147,8 @@ public final class MusicRecognitionManagerPerUserService extends
Pair<ParcelFileDescriptor, ParcelFileDescriptor> clientPipe = createPipe();
if (clientPipe == null) {
try {
mCallback.onAudioStreamClosed();
clientCallback.onRecognitionFailed(
RECOGNITION_FAILED_AUDIO_UNAVAILABLE);
} catch (RemoteException ignored) {
// Ignored.
}
@@ -192,11 +193,10 @@ public final class MusicRecognitionManagerPerUserService extends
} finally {
audioRecord.release();
try {
mCallback.onAudioStreamClosed();
clientCallback.onAudioStreamClosed();
} catch (RemoteException ignored) {
// Ignored.
}
}
});
// Send the pipe down to the lookup service while we write to it asynchronously.
@@ -207,13 +207,20 @@ public final class MusicRecognitionManagerPerUserService extends
* Callback invoked by {@link android.service.musicrecognition.MusicRecognitionService} to pass
* back the music search result.
*/
private final class MusicRecognitionServiceCallback extends
final class MusicRecognitionServiceCallback extends
IMusicRecognitionServiceCallback.Stub {
private final IMusicRecognitionManagerCallback mClientCallback;
private MusicRecognitionServiceCallback(IMusicRecognitionManagerCallback clientCallback) {
mClientCallback = clientCallback;
}
@Override
public void onRecognitionSucceeded(MediaMetadata result, Bundle extras) {
try {
sanitizeBundle(extras);
mCallback.onRecognitionSucceeded(result, extras);
mClientCallback.onRecognitionSucceeded(result, extras);
} catch (RemoteException ignored) {
// Ignored.
}
@@ -223,18 +230,23 @@ public final class MusicRecognitionManagerPerUserService extends
@Override
public void onRecognitionFailed(@RecognitionFailureCode int failureCode) {
try {
mCallback.onRecognitionFailed(failureCode);
mClientCallback.onRecognitionFailed(failureCode);
} catch (RemoteException ignored) {
// Ignored.
}
destroyService();
}
private IMusicRecognitionManagerCallback getClientCallback() {
return mClientCallback;
}
}
@Override
public void onServiceDied(@NonNull RemoteMusicRecognitionService service) {
try {
mCallback.onRecognitionFailed(RECOGNITION_FAILED_SERVICE_KILLED);
service.getServerCallback().getClientCallback().onRecognitionFailed(
RECOGNITION_FAILED_SERVICE_KILLED);
} catch (RemoteException e) {
// Ignored.
}

View File

@@ -22,7 +22,9 @@ import static android.media.musicrecognition.MusicRecognitionManager.RECOGNITION
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.ComponentName;
import android.content.Context;
import android.content.pm.PackageManager;
import android.media.musicrecognition.IMusicRecognitionManager;
import android.media.musicrecognition.IMusicRecognitionManagerCallback;
import android.media.musicrecognition.RecognitionRequest;
@@ -32,7 +34,9 @@ import android.os.RemoteException;
import android.os.ResultReceiver;
import android.os.ShellCallback;
import android.os.UserHandle;
import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
import com.android.server.infra.AbstractMasterSystemService;
import com.android.server.infra.FrameworkResourcesServiceNameResolver;
@@ -113,9 +117,11 @@ public class MusicRecognitionManagerService extends
enforceCaller("beginRecognition");
synchronized (mLock) {
int userId = UserHandle.getCallingUserId();
final MusicRecognitionManagerPerUserService service = getServiceForUserLocked(
UserHandle.getCallingUserId());
if (service != null) {
userId);
if (service != null && (isDefaultServiceLocked(userId)
|| isCalledByServiceAppLocked("beginRecognition"))) {
service.beginRecognitionLocked(recognitionRequest, callback);
} else {
try {
@@ -139,5 +145,55 @@ public class MusicRecognitionManagerService extends
MusicRecognitionManagerService.this).exec(this, in, out, err, args, callback,
resultReceiver);
}
/** True if the currently set handler service is not overridden by the shell. */
@GuardedBy("mLock")
private boolean isDefaultServiceLocked(int userId) {
final String defaultServiceName = mServiceNameResolver.getDefaultServiceName(userId);
if (defaultServiceName == null) {
return false;
}
final String currentServiceName = mServiceNameResolver.getServiceName(userId);
return defaultServiceName.equals(currentServiceName);
}
/** True if the caller of the api is the same app which hosts the default service. */
@GuardedBy("mLock")
private boolean isCalledByServiceAppLocked(@NonNull String methodName) {
final int userId = UserHandle.getCallingUserId();
final int callingUid = Binder.getCallingUid();
final String serviceName = mServiceNameResolver.getServiceName(userId);
if (serviceName == null) {
Slog.e(TAG, methodName + ": called by UID " + callingUid
+ ", but there's no service set for user " + userId);
return false;
}
final ComponentName serviceComponent = ComponentName.unflattenFromString(serviceName);
if (serviceComponent == null) {
Slog.w(TAG, methodName + ": invalid service name: " + serviceName);
return false;
}
final String servicePackageName = serviceComponent.getPackageName();
final PackageManager pm = getContext().getPackageManager();
final int serviceUid;
try {
serviceUid = pm.getPackageUidAsUser(servicePackageName,
UserHandle.getCallingUserId());
} catch (PackageManager.NameNotFoundException e) {
Slog.w(TAG, methodName + ": could not verify UID for " + serviceName);
return false;
}
if (callingUid != serviceUid) {
Slog.e(TAG, methodName + ": called by UID " + callingUid + ", but service UID is "
+ serviceUid);
return false;
}
return true;
}
}
}

View File

@@ -21,13 +21,13 @@ import android.content.ComponentName;
import android.content.Context;
import android.media.AudioFormat;
import android.media.musicrecognition.IMusicRecognitionService;
import android.media.musicrecognition.IMusicRecognitionServiceCallback;
import android.media.musicrecognition.MusicRecognitionService;
import android.os.IBinder;
import android.os.ParcelFileDescriptor;
import android.text.format.DateUtils;
import com.android.internal.infra.AbstractMultiplePendingRequestsRemoteService;
import com.android.server.musicrecognition.MusicRecognitionManagerPerUserService.MusicRecognitionServiceCallback;
/** Remote connection to an instance of {@link MusicRecognitionService}. */
public class RemoteMusicRecognitionService extends
@@ -39,11 +39,12 @@ public class RemoteMusicRecognitionService extends
private static final long TIMEOUT_IDLE_BIND_MILLIS = 40 * DateUtils.SECOND_IN_MILLIS;
// Allows the remote service to send back a result.
private final IMusicRecognitionServiceCallback mServerCallback;
private final MusicRecognitionServiceCallback
mServerCallback;
public RemoteMusicRecognitionService(Context context, ComponentName serviceName,
int userId, MusicRecognitionManagerPerUserService perUserService,
IMusicRecognitionServiceCallback callback,
MusicRecognitionServiceCallback callback,
boolean bindInstantServiceAllowed, boolean verbose) {
super(context, MusicRecognitionService.ACTION_MUSIC_SEARCH_LOOKUP, serviceName, userId,
perUserService,
@@ -66,6 +67,10 @@ public class RemoteMusicRecognitionService extends
return TIMEOUT_IDLE_BIND_MILLIS;
}
MusicRecognitionServiceCallback getServerCallback() {
return mServerCallback;
}
/**
* Required, but empty since we don't need to notify the callback implementation of the request
* results.