commit ab3d5f7ba8cbc0bbc8f6c99f1bbf53efb9c8cdfe
author Matt Maynard <mmayna26@ford.com> Wed Mar 01 20:05:11 2023 -0500
committer Matt Maynard <mmayna26@ford.com> Tue Feb 06 14:11:13 2024 -0500
tree 02bfe113d495f4dcb04072e08d232be4b747b35f
parent e94440dc096db9244499fd55df6a241f203ab6bd

Harden playback state checks against IPC race

Bug: 271413253

Automated crash detection found a NPE of the form:

Process: com.android.car
PID: 1634
UID: 1000
Flags: 0x30883e4d
Package: com.android.car v32 (12)
Foreground: No
Process-Runtime: 104844637
Build: Phoenix/belford_gas/belford:12/V1.0.0-2022.07-130/PU5T-14H648-QAA5:userdebug/dev-keys
Loading-Progress: 1.0

java.lang.NullPointerException: Attempt to invoke virtual method 'int android.media.session.PlaybackState.getState()' on a null object reference
	at com.android.car.CarMediaService.updatePrimaryMediaSourceWithCurrentlyPlaying(CarMediaService.java:887)
	at com.android.car.CarMediaService.access$2000(CarMediaService.java:95)
	at com.android.car.CarMediaService$MediaSessionUpdater.registerCallbacks(CarMediaService.java:759)
	at com.android.car.CarMediaService$MediaSessionUpdater.access$1300(CarMediaService.java:726)
	at com.android.car.CarMediaService$SessionChangedListener.onActiveSessionsChanged(CarMediaService.java:685)
	at android.media.session.MediaSessionManager$SessionsChangedWrapper.callOnActiveSessionsChangedListener(MediaSessionManager.java:1287)
	at android.media.session.MediaSessionManager$SessionsChangedWrapper.access$600(MediaSessionManager.java:1255)
	at android.media.session.MediaSessionManager$SessionsChangedWrapper$1.lambda$onActiveSessionsChanged$0$MediaSessionManager$SessionsChangedWrapper$1(MediaSessionManager.java:1272)
	at android.media.session.MediaSessionManager$SessionsChangedWrapper$1$$ExternalSyntheticLambda0.run(Unknown Source:4)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loopOnce(Looper.java:201)
	at android.os.Looper.loop(Looper.java:288)
	at android.os.HandlerThread.run(HandlerThread.java:67)

Root Cause:

CarMediaService uses an unsafe pattern of NULL checking when the
getPlaybackState() call is a binder IPC call.  The first null check with

  controller.getPlaybackState()

returns a non-null value and then the next line calls the same IPC call
again :

  controller.getPlaybackState().isActive()

but from the 1st IPC call to the 2nd, the state in the MediaSession
process has changed and so the 2nd call returns NULL from
getPlaybackState().

race condition diagram looks like :

CarService/MediaController Process  | ==|         MediaSession Pocess
----------------------------------------------------------------------
1st IPC call:

  controller.getPlaybackState()    --->

                                   <---             return non-NULL

  result is non-NULL, proceed.

                                                    change in media
                                                    process such that
                                                    the value is now
                                                    NULL

2nd IPC call:

  controller.getPlaybackState()    --->

                                   <--              return NULL

  result.isActive() ->   NULL deref causes crash on 2nd IPC execution

Make one single IPC call into a local state variable (like the rest of
the file) and use that local variable for the 2nd operation so that the
value can not change between uses.

This change hardens the playback state changes to protect against
potential NULL dereference path in small but real time window caused by
IPC callss.

Change-Id: Ibfb4dedc3a66fc4a789b764117e93f80bb8b7872
Signed-off-by: Matt Maynard <mmayna26@ford.com>