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

Skip to content
Commit 4696c675 authored by Jay Thomas Sullivan's avatar Jay Thomas Sullivan Committed by Cherrypicker Worker
Browse files

Fix privacy indicators leak (i.e., active op leak)

This change fixes an intermittent bug where privacy indicator icons will
sometimes stay on, even after the last app actively using the appop
(e.g., camera, microphone, or location) terminates.

Background:

We begin tracking that an app is actively using one of these appops as
soon as startOp (or another startOp-like method) is called. We stop
tracking whenever one of the following triggers occur (whichever
happens first):

1. finishOp (or another finishOp-like method) is explicity called)
    (invoking AppOpsService::finishOperation)
2. The IBinder token, a required arg to startOp, dies
    (invoking AppOpsService::onClientDeath)

Both triggers end up calling the following chain of functions, which
results in the privacy indicator visibly disappearing:

    AttributedOp::finished
        -> AttributedOp::finishOrPaused,
            -> AppOpsService:scheduleOpActiveChangedIfNeededLocked
                -> ...
                    -> (indicator visibly disappears)

The problem:

The problem occurs inside another function,
AttributedOp::onUidStateChanged (triggered whenever a currently-
active-op's uid changes uid state) performs the unique behavior of
"restarting" an active op, which involves finishing the active op
(AttributedOp::finished) and then starting it again
(AttributedOp::startedOrPaused).

In order for onUidStateChanged to perform this restart functionality
without causing chaos, when it finishes an active-op, it passes a flag
to indicate that active watchers should not be notified of this
operation (triggerCallbackIfNeeded=false).

The problem is that active ops are keyed by live IBinder sessions.
And, a binder session can die at any time. So, if the binder dies during
the brief moment in AttributedOp::onUidStateChanged between the
finished() and startedOrPaused() calls, then, startedOrPaused() will
throw a DeadObjectException, because it needs to interact with the
binder (to register an binder.onClientDeath callback). In this case, the
active op will be left finished, but it never actually notified any
watchers. To the user, the state of the indicator appears to have
leaked.

The solution:

When a DeadObjectException is thrown from startedOrPaused(),
notify the watchers that the active op is finished. Also, rename the
triggerCallbackIfNeeded argument to reduce the risk of future users
repeating this mistake.

Fix: 290091092
Test: cd packages/modules/Permission && atest :alltests
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5d7e9f78bbaa02f98ac5edb36da10404de23f305)
Merged-In: I48f5577f7c611eccdd803a2a9c32790362a329ea
Change-Id: I48f5577f7c611eccdd803a2a9c32790362a329ea
parent 12fd5027
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment