Skip to content

Fix refcount race, flush() races, callerPkg spoofing#280

Open
thomasbuilds wants to merge 3 commits into
GrapheneOS:16-qpr2from
thomasbuilds:fix
Open

Fix refcount race, flush() races, callerPkg spoofing#280
thomasbuilds wants to merge 3 commits into
GrapheneOS:16-qpr2from
thomasbuilds:fix

Conversation

@thomasbuilds

Copy link
Copy Markdown

Proper #275. The reasoning for each change is in the commit bodies, with all framework behavior claims checked against the 16-qpr2 frameworks/base sources.

binderDied() calls PersistentFgService.release(), which could run before its matching
requestStart(): the RemoteException catch path invoked it directly before requestStart() was
reached, and an early asynchronous death notification could post CMD_MAYBE_STOP to the main
thread handler before connect() posted CMD_START. An early release can stop the service while
other GMS processes are still bound, or underflow the refcount, which crashes the GmsCompat app.
Requesting the start before linkToDeath() guarantees that CMD_START is always enqueued first.
If the listener gets unregistered after requestFlush() succeeds but before onFlushComplete() is
delivered, the OS drops the callback (see LocationManager#requestFlush() javadoc), leaving the
unbounded await() blocked forever. If unregistration wins the race instead, requestFlush() throws
IllegalArgumentException ("unregistered listener cannot be flushed"), not the currently caught
IllegalStateException, which is thrown when the provider isn't started. Both windows are
reachable: flushLocations() iterates over listeners outside of the listeners lock, and the OS
removes registrations on its own when maxUpdates or durationMillis are exhausted.

The wait is now bounded at 5 seconds, and a request code lets onFlushComplete() ignore a late
callback from a previous timed-out flush. The code and latch are published through a single
volatile field since onFlushComplete() runs without holding the monitor.
startActivityFromTheBackground() and showMissingPostNotifsPermissionNotification() used the
caller-provided package name unverified for notification labels and per-package notification
state, letting a GmsCompat-enabled app post notifications under another app's name.
@thomasbuilds

Copy link
Copy Markdown
Author

cc @muhomorr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant