AudioService playback activity notif: check origin of player updates

When receiving updates about players, check the validity of the call:
 - the piid must be valid
 - the uid of the caller and that of the player must match

Test: adb shell dumpsys audio
Bug: 30955183
Change-Id: Id15e2b69764ed7db0b93e2c0c96472c30b1d2070

diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java
index a50f722..2d42c33 100644 (file)
--- a/services/core/java/com/android/server/audio/AudioService.java
+++ b/services/core/java/com/android/server/audio/AudioService.java
@@ -6491,7 +6491,7 @@ public class AudioService extends IAudioService.Stub
     }
 
     //======================
-    // Audio policy callbacks from players for playback configuration updates
+    // Audio playback notification
     //======================
     private final PlaybackActivityMonitor mPlaybackMonitor = new PlaybackActivityMonitor();
 
@@ -6518,15 +6518,15 @@ public class AudioService extends IAudioService.Stub
     }
 
     public void playerAttributes(int piid, AudioAttributes attr) {
-        mPlaybackMonitor.playerAttributes(piid, attr);
+        mPlaybackMonitor.playerAttributes(piid, attr, Binder.getCallingUid());
     }
 
     public void playerEvent(int piid, int event) {
-        mPlaybackMonitor.playerEvent(piid, event);
+        mPlaybackMonitor.playerEvent(piid, event, Binder.getCallingUid());
     }
 
     public void releasePlayer(int piid) {
-        mPlaybackMonitor.releasePlayer(piid);
+        mPlaybackMonitor.releasePlayer(piid, Binder.getCallingUid());
     }
 
     //======================

diff --git a/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java b/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java
index dfa856d..d6b4bee 100644 (file)
--- a/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java
+++ b/services/core/java/com/android/server/audio/PlaybackActivityMonitor.java
@@ -78,15 +78,15 @@ public final class PlaybackActivityMonitor {
         return newPiid;
     }
 
-    public void playerAttributes(int piid, @NonNull AudioAttributes attr) {
+    public void playerAttributes(int piid, @NonNull AudioAttributes attr, int binderUid) {
         final boolean change;
         synchronized(mPlayerLock) {
             final AudioPlaybackConfiguration apc = mPlayers.get(new Integer(piid));
-            if (apc == null) {
-                Log.e(TAG, "Unknown player " + piid + " for audio attributes change");
-                change = false;
-            } else {
+            if (checkConfigurationCaller(piid, apc, binderUid)) {
                 change = apc.handleAudioAttributesEvent(attr);
+            } else {
+                Log.e(TAG, "Error updating audio attributes");
+                change = false;
             }
         }
         if (change) {
@@ -94,17 +94,17 @@ public final class PlaybackActivityMonitor {
         }
     }
 
-    public void playerEvent(int piid, int event) {
+    public void playerEvent(int piid, int event, int binderUid) {
         if (DEBUG) { Log.v(TAG, String.format("playerEvent(piid=%d, event=%d)", piid, event)); }
         final boolean change;
         synchronized(mPlayerLock) {
             final AudioPlaybackConfiguration apc = mPlayers.get(new Integer(piid));
-            if (apc == null) {
-                Log.e(TAG, "Unknown player " + piid + " for event " + event);
-                change = false;
-            } else {
+            if (checkConfigurationCaller(piid, apc, binderUid)) {
                 //TODO add generation counter to only update to the latest state
                 change = apc.handleStateEvent(event);
+            } else {
+                Log.e(TAG, "Error handling event " + event);
+                change = false;
             }
         }
         if (change) {
@@ -112,13 +112,14 @@ public final class PlaybackActivityMonitor {
         }
     }
 
-    public void releasePlayer(int piid) {
+    public void releasePlayer(int piid, int binderUid) {
         if (DEBUG) { Log.v(TAG, "releasePlayer() for piid=" + piid); }
         synchronized(mPlayerLock) {
-            if (!mPlayers.containsKey(new Integer(piid))) {
-                Log.e(TAG, "Unknown player " + piid + " for release");
-            } else {
+            final AudioPlaybackConfiguration apc = mPlayers.get(new Integer(piid));
+            if (checkConfigurationCaller(piid, apc, binderUid)) {
                 mPlayers.remove(new Integer(piid));
+            } else {
+                Log.e(TAG, "Error releasing player " + piid);
             }
         }
     }
@@ -133,6 +134,25 @@ public final class PlaybackActivityMonitor {
         }
     }
 
+    /**
+     * Check that piid and uid are valid for the given configuration.
+     * @param piid the piid of the player.
+     * @param apc the configuration found for this piid.
+     * @param binderUid actual uid of client trying to signal a player state/event/attributes.
+     * @return true if the call is valid and the change should proceed, false otherwise.
+     */
+    private static boolean checkConfigurationCaller(int piid,
+            final AudioPlaybackConfiguration apc, int binderUid) {
+        if (apc == null) {
+            Log.e(TAG, "Invalid operation: unknown player " + piid);
+            return false;
+        } else if ((binderUid != 0) && (apc.getClientUid() != binderUid)) {
+            Log.e(TAG, "Forbidden operation from uid " + binderUid + " for player " + piid);
+            return false;
+        }
+        return true;
+    }
+
     private void dispatchPlaybackChange() {
         synchronized (mClients) {
             // typical use case, nobody is listening, don't do any work