Give back the content provider reference we take to deliver a module's service - #893
Merged
Conversation
`IActivityManager.getContentProviderExternal` gained its `tag` argument in Q, and the AIDL replaced the three-argument form rather than overloading it: there is exactly one declaration of that method at every API level, so a call written for either shape is a NoSuchMethodError on the other side of that line. The daemon has called the four-argument form unconditionally since #597: the branch lived in `ActivityManagerService.getContentProvider`, which that commit deleted along with the rest of the Java daemon, and the Kotlin that replaced it asked for the four-argument form outright. The consequence is that no module built against libxposed has been handed its app-side `IXposedService` on Android 8.1 or 9 for as long as that has been true, and nothing said so. The daemon compiles against a hand-written stub, so the wrong arity survives the build; at run time the receiver is the platform's own `IActivityManager$Stub$Proxy`, the error lands in the `runCatching` around the delivery, and the one line it produces is the same one an ordinary failed delivery prints. Hooking is unaffected -- a module's injected-process service comes from elsewhere -- so what this cost was the module's own settings UI reporting the framework as absent while its hooks worked. `minSdk` is 27, so the older form has to stay.
…ting one that will not start Handing a module app its service means starting it: `getContentProviderExternal` on the module's own `XposedProvider` brings the process up and waits for it to publish. The reference that takes was never given back, and the platform reads an outstanding external reference as a live client of the provider. Two things follow, and both have been true of every module using the libxposed API, on Q and above -- legacy modules are never handed a binder at all, and the last paragraph here explains why Q. The host is pinned. `OomAdjuster.computeOomAdjLSP` raises a process publishing such a provider to FOREGROUND_APP_ADJ and PROCESS_STATE_IMPORTANT_FOREGROUND -- `adjType=ext-provider` -- so a module app was held at foreground priority for as long as it lived, never cached and never trimmed. Measured on a Galaxy A52s: a backgrounded module app sat at `Proc #1: fg F/ /IMPF (ext-provider)` with adj 0 and procstate 6, above everything but the launcher, while the manager beside it was cached. With the reference given back it goes to `cch-act`, adj 900, procstate 16. A uid kept out of the background is also a uid that keeps being reported active, and the delivery is driven by exactly those uid callbacks, so it woke itself. The host is also restarted. A process that dies while a provider of its is still launching is restarted for it, three times per provider record, after which the platform drops the record and gives up -- but taking the reference again builds a fresh record with the count back at zero. That is how a bounded retry became unbounded in #889, where a module in a third module's scope killed its host on every launch: fourteen starts in seventy-six seconds, six of them ours. The release has to be unconditional to fix it. The platform registers the external client before it waits for the app to publish, so the two returns that matter -- the app died while launching, and the wait timed out -- come back null with the reference still held; releasing only when a provider came back would have left the loop exactly as it was. Asking when nothing was registered is not free of consequence, only of harm: if the app is not running there is no record and the platform returns quietly, and if it is, the platform logs that something tried to remove a reference it does not have. A line in its log against the loop this stops is the right side of that trade. A real token instead of null earns a death link, so system_server drops the reference when the daemon dies rather than holding it for the life of the record. The old null was only a counter, with nothing to link. The record of a delivery is also per process now, not per uid. A binder belongs to the process that received it, but a uid outlives any one of its processes -- an app with a `:remote` or crash process, or a shared user id, keeps its uid alive when the process we served is reaped -- so no uid death arrives and the replacement process would be refused for ever. Nothing reaped a module app while it was pinned, which is what kept that unreachable; unpinning it makes it the ordinary case. So a successful delivery is watched, and the process dying is what makes the next one eligible. The delivery no longer runs on the uid observer. `getContentProviderExternal` blocks until the app publishes or the platform stops waiting, and one observer thread serves every uid transition on the device, so a module app that never publishes stalled the delivery of every other module's binder behind it -- eight and a half seconds in that report, against two hundred milliseconds once the crashing module was disabled. And the retries have a memory. A failed send used to be recorded exactly as a delivered one, so the module that most needed another attempt never got one, while the module that could not take its binder at all was asked again a second later for as long as it kept dying. Success is what marks the uid now; failures are counted per uid and throttled to one attempt a minute after three in a row, counted from the last failure so an occasional one never accumulates, and cleared by the first success -- an app can equally have been mid-update, and a module written off for good on three failures would be the worse bug. The count is held at the ceiling rather than reset by the attempt the cooldown lets through, or the ceiling would be re-climbed and three more attempts allowed every minute for ever; and the run is kept per uid, since one module installed for two users is one module under two uids and a crash-looping copy in a work profile must not throttle the healthy one in user 0. The release side needs a version test of its own. `removeContentProviderExternal` gained its user id in Q, not R as the gate first said, so on Android 10 the deprecated two-argument form was used and AMS resolved the authority against the daemon's own user -- finding nothing, and saying nothing. On 27 and 28 that form is all there is, and a reference taken for a module in a secondary user cannot be given back at all; asking anyway would decrement the token-less counter of whatever record user 0 has under that name, so there it is left to the token's death link instead.
`IUidObserver` is a hidden interface, so the daemon compiles against a stub of our own and extends the framework's real class at runtime. The stub declared four of its methods. Every one it left out is a method the observer cannot override and the platform still finds abstract, and the interface is `oneway`, so nothing waits for the call and nothing catches what it throws: an AbstractMethodError there is routed by `JavaBBinder::onTransact` to `report_java_lang_error`, which is fatal, and the daemon's own uncaught handler would end the process regardless. It has never fired. `onUidStateChanged` and `onUidProcAdjChanged` are gated on UID_OBSERVER_PROCSTATE, UID_OBSERVER_CAPABILITY and UID_OBSERVER_PROC_OOM_ADJ, and the registration asks for ACTIVE, GONE, IDLE and CACHED. That is the whole of the guarantee -- one line in this file away from a daemon that dies on the first uid transition after boot, on a device nobody can hold. So the stub now carries every method the interface has had from API 27 to 37, and the observer overrides them. Two of them changed shape inside that range and both forms are declared, since only one exists per release and the other is then an unused method: `onUidStateChanged` gained its capability argument in 30, and `onUidProcAdjChanged` arrived in 33 taking a uid and gained its adj in 34.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A module app does not receive its
IXposedServicethrough a binding.ModuleAppService.sendBinderasksgetContentProviderExternalfor the module's own<pkg>.XposedServiceauthority — which starts the process if it is not running and blocks until it publishes — and then calls the provider with the binder in aBundle. The reference that acquisition takes was never given back, and that turns out to be the interesting part.The platform reads an outstanding external reference as a live client of the provider.
ContentProviderRecord.hasExternalProcessHandles()isexternalProcessTokenToHandle != null || externalProcessNoHandleCount > 0— it counts references taken with a token and without one — so a single un-released acquisition is enough to make it true for the life of the record. Two conclusions follow from that, by two different routes.The first is that the host is pinned.
OomAdjuster.computeOomAdjLSPtests it directly and raises a process publishing such a provider toFOREGROUND_APP_ADJandPROCESS_STATE_IMPORTANT_FOREGROUNDand records the reason asext-provider. This has been true of every module app on every device, not only of one that misbehaves: we take a reference to hand over a binder and never release it, so the app is held at foreground priority for as long as it lives, never cached and never trimmed. On Android 14 a backgrounded module app sat atone row of
dumpsys activity processes, second in its section behind only the launcher. That section is headedProcess LRU listbut, as the header goes on to say, it is sorted by oom_adj — so a row's position in it is a ranking by protection, while the#Nin the row is something else: the process's place in the LRU list proper, which is AMS's running processes ordered by how recently each was used.adjis the out-of-memory adjustment AMS computes for a process and writes to the kernel, and whichlmkdkills in descending order of when memory runs short.ProcessListnames the bands —FOREGROUND_APP_ADJis 0,CACHED_APP_MIN_ADJis 900 — so lower is harder to kill, andcur=0is as protected as an ordinary app gets.fgis that band's label. The rest of the row:F/is the scheduling group;IMPFis the process state, short forPROCESS_STATE_IMPORTANT_FOREGROUND; and(ext-provider)is the adj type, the stringOomAdjusterrecords to say why a process was raised — here, that something holds an external reference to a provider it publishes. The manager beside it wascch, cached.With the reference given back the same app is
cch-act— cached, kept only for the activity it has — at adj 900 and process state 16,PROCESS_STATE_CACHED_ACTIVITY. A module app with no components of its own becomescch-empty, a cached empty process, which is the lowest thing on the list and the first to be reclaimed.The second is that the host is restarted. A process that dies while a provider of its is still launching is restarted for it —
ProcessProviderRecord.onCleanupApplicationRecordLockedandContentProviderHelper.cleanupAppInLaunchingProvidersLockedboth decide onhasConnectionOrHandle(), which is!connections.isEmpty() || hasExternalProcessHandles()— and the platform bounds that atMAX_RETRY_COUNT = 3per provider record, after which it drops the record and gives up. Acquiring again builds a fresh record with the count back at zero. So the platform's retry is bounded and ours was not. In #889 the app was started fourteen times in seventy-six seconds, and six of those were the daemon — each arriving ascontent provider,{…/io.github.libxposed.service.XposedProvider}withcallingPackage = null, which is this acquisition.What kept killing it is worth naming, because it is the reason the loop had anything to feed on.
me.feimeng.vip— published as a binary, no sources — readsgetFrameworkName()in its API 101 entry point, compares it against an allowlist, and on no match calls a JNI-registered native method whose entire body isa store through a deliberately zeroed register. The module is not what dies:
onModuleLoadedruns inside whichever process the module was injected into, so the host goes down withsignal 11 (SIGSEGV) … fault addr 0x0before it has finished binding its application — every tombstone readsCmdline: <pre-initialized>. Disassembling the published 3.8.1, 3.9.0 and 3.9.7.3 gives an allowlist of{LSPosed, FPA}in the first two and{LSPosed, FPA, NPatch}in the third, with the string obfuscation re-randomised per release and the null store itself MBA-obfuscated in the newest, so it is maintained rather than incidental. The build on the reporting device is newer than anything published there and could not be obtained, but every tombstone's frame #00 is in itslibfeimeng.so, called fromme.feimeng.vip.loader.lsp101.ModuleApi.onModuleLoaded, withx8zeroed and the fault address zero.None of this is new to the tracker. The module was the first name in #816, reported as activating on LSPosed and not on Vector, and the disassembly above dates from that investigation; #819 fixed the unrelated legacy self-scope half of the same report. What #889 adds is that the cost is no longer confined to the module that chose it — the process it takes down belongs to somebody else, and the framework then puts it back up.
The reporter's own logs carry the control. On the same device, a local fork that renamed the framework "LSPosed" recorded no crashes at all; the builds reporting "Vector" recorded twenty-six and thirteen. Nothing here can be defended against — the framework already wraps
onModuleLoadedinrunCatching, and a SIGSEGV in a module's own JNI is not catchable — so the only thing in scope is to stop being the one that restarts the corpse.Releasing only on success would not have fixed that. The platform registers the external client in
incProviderCountLockedbefore it waits for the app to publish, and the two returns that matter — the launching app died, and the wait timed out — come back null with the reference still held. Those are precisely the returns a module app that dies on every start produces, so the release has to be unconditional. Asking when nothing was registered is not free of consequence, only of harm: if the app is not running there is no record and the call returns quietly, and if it is, the platform logs that something tried to remove a reference it does not hold. A line in its log against this loop is the right side of that trade.Passing a real token rather than
nullmatters for the same reason. The platform builds anExternalProcessHandlearound a token and links to its death, so system_server drops the reference when the daemon dies rather than holding it for the life of the record; a null token is only a counter, with nothing to link, which is why the old leak was permanent rather than merely long.Three consequences of the delivery path fall out of the same investigation.
The delivery no longer runs on the uid observer.
getContentProviderExternalblocks until the app publishes or the platform stops waiting for it, and one binder thread serves every uid transition on the device, so a module app that never publishes stalls the delivery of every other module's binder behind it. Measured on the device in #889: eight and a half seconds fromam_uid_activetoSent module binder, against two hundred milliseconds once the crash-looping module was disabled. The same log carries sixam_wtfentries — the tag AMS uses for a condition it considers a bug worth reporting — each readingTimeout waiting for provider … caller=unknown/1000, i.e. attributed to the daemon.The retries have a memory. A failed send used to be recorded exactly as a delivered one —
uidSet.addran before the attempt — so the module that most needed a second attempt never got one, while the module that could not take its binder at all was asked again a second later for as long as it kept dying. Since the acquisition starts the process, retrying is not a passive act; it feeds the loop it is failing on. Success now marks the uid, failures are counted per uid, and after three in a row the retries are throttled to one a minute. Throttled rather than abandoned, and cleared by the first success: an app can equally have been mid-update or out of memory, and a module written off for good on three failures would be the worse bug.And the record of a delivery is per process rather than per uid. A binder belongs to the process that received it, but a uid outlives any one of its processes — an app with a
:remoteor crash-handler process, or a shared user id, keeps its uid alive when the served process is reaped, so noonUidGonearrives and the replacement process would be refused for ever. Nothing reaped a module app while it was pinned, which is exactly what kept this unreachable; unpinning them makes it the ordinary case. A successful delivery is now watched, and the death of the process that took the binder is what makes the next one eligible.Two version fixes are separate commits.
getContentProviderExternalgained itstagargument in Q, and the AIDL replaced the three-argument form rather than overloading it — 9.0 declares only the three-argument one — so a call written for either shape is aNoSuchMethodErroron the other side of that line. The daemon has called the four-argument form unconditionally since #597: the branch lived inActivityManagerService.getContentProvider, which that commit deleted along with the rest of the Java daemon, and the Kotlin that replaced it asked for the four-argument form outright. No module built against libxposed has been handed its app-side service on 8.1 or 9 since, and nothing said so: the stub is hand-written so the arity survives the build, and at run time the error lands in therunCatchingaround the delivery and prints the same line an ordinary failed delivery does. Hooking is unaffected, so what this cost was a module's own settings UI reporting the framework as absent while its hooks worked.removeContentProviderExternalAsUseralso arrived in Q, not R, and the two-argument form it deprecates resolves the authority against the caller's user. On 27 and 28 that form is all there is, so a reference taken for a module in a secondary user cannot be given back at all — asking anyway would decrement the token-less counter of whatever record user 0 has under that name — and there it is left to the token's death link instead.The last commit completes the
IUidObserverstub. It declared four of the interface's methods; across 27 to 37 there are eight, becauseonUidStateChangedgained acapabilityargument in 30 andonUidProcAdjChangedarrived in 33 taking a uid and gained anadjin 34. Since the real class is what aStubsubclass extends at run time, a method missing from the stub is one the subclass cannot override and the platform still finds abstract. It has never fired —UidObserverControllergates both onUID_OBSERVER_PROCSTATE,UID_OBSERVER_CAPABILITYandUID_OBSERVER_PROC_OOM_ADJ, and we register forACTIVE|GONE|IDLE|CACHED— but that is the whole of the guarantee, and the failure is not survivable: the interface isoneway, so nothing waits and nothing catches, andJavaBBinder::onTransacthands what escapes tobinder_report_exception, which sends ajava.lang.Erroron toreport_java_lang_error— and that aborts. Both shapes of each are declared; only one exists per release and the other is an unused method on the subclass.Verified on Android 14, with the minimal module added to
api102-harnessin c162e66 — none of the modules to hand ship libxposed-service, so none can act as a counterparty. The module logs the binder it was given at 20:50:07.909 and the daemon logsSent module binderat 20:50:07.910, in that order, which is what makes releasing immediately safe:onBinderReceivedcompletes beforecallreturns. Per-process invalidation was checked by holding the uid open with a second process, killing the one that held the binder —am_uid_stopped, the event AMS logs when a uid loses its last process and the one that drivesonUidGone, never appeared for that uid — and observing a second delivery to the replacement. The throttle was driven by disabling the module's provider so the acquisition fails without needing a crashing app: six triggers produced three attempts and one warning, and eight more over the following cooldown produced exactly one.One behaviour change is worth a release note. Module apps are no longer immortal, so a scope-request answer arriving up to an hour later can now find the app gone. The grant itself is safe —
ModuleDatabase.setModuleScopeis written beforeonScopeRequestApproved— but a module that refreshes its UI in that callback may not see the change until it is next launched.