Repository navigation
Conversation
The after-callback on security_path_rename/security_inode_rename ran the whole trusted-manager refresh synchronously: /data/app scan, APK signature read and packages.list(.tmp) read. It runs inside do_renameat2()/vfs_rename() with the parent directory i_rwsem held for write. Any lookup in that directory that misses the dcache goes through lookup_slow()/open_last_lookups() and down_read()s the same i_rwsem, and the task deadlocks against itself. In 0.13.3/0.13.4 the packages.xml fallback opened /data/system/packages.xml from there, which wedged system_server (PackageManager in D state in path_openat -> down_read under vfs_rename) during Play Store batch updates. HEAD no longer reads packages.xml, but it still does all of the I/O while holding /data/system exclusively. Queue the refresh with task_work_add(current, TWA_RESUME) instead, so it runs when the renaming task returns to user space, after every VFS lock is dropped, in the same task and credentials as before. It now reads the final packages.list (use_tmp=0). If task_work_add cannot be resolved or fails, skip the refresh and log it; the uid_listener event and reload-cfg still refresh from process context. Only uid 1000 (system_server) triggers the refresh. Before this, any app could trigger the scan by renaming its own .../system/packages.list.tmp. All trusted-manager refreshes are serialized through a running/again flag pair. A refresh requested while one is in flight makes the runner loop again, so an older scan cannot overwrite a newer result. Also drop the stale "fallback to xml" wording from the log. The xml fallback was removed in a9224bf.
|
This isn't a duplicate of #322. #322 fixes which function gets hooked on LTO kernels. #326 fixes the deadlock from doing the refresh I/O under the rename's As noted in the PR description, the two don't conflict and both are needed: #322's #327 is separate from both. It fixes a second deadlock in the Could you reopen this? |
There was a problem hiding this comment.
🟡 Changes recommended
The serialization can lose refresh requests, slot exhaustion can miss the latest rename, and synchronous callers may receive premature success.
3 open findings
What changed in this PR
Defers trusted-manager refreshes until after rename locks are released, preventing VFS lock self-deadlocks.
Changes:
- Queues refreshes through
task_work_add(). - Restricts rename-triggered refreshes to system UID 1000.
- Adds refresh serialization and updates logging.
| File | Description |
|---|---|
kernel/patch/android/userd.c |
Implements deferred, serialized trusted-manager refreshes. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| smp_store_release(&trusted_manager_refresh_again, 1); | ||
| if (!smp_load_acquire(&trusted_manager_refresh_running)) continue; |
| log_boot("trusted manager refresh coalesced\n"); | ||
| return 0; |
| /* every slot is queued on another renaming task; its refresh runs again */ | ||
| smp_store_release(&trusted_manager_refresh_again, 1); | ||
| log_boot("packages.list rename: refresh slots busy, coalesced\n"); |
Summary
Queue the trusted-manager refresh with
task_work_add()instead of running it inside the rename LSM hook, so no file I/O happens while/data/systemis locked by the rename.Problem
after_security_path_rename/after_security_inode_renamerun insidevfs_rename()with the parent directoryi_rwsemheld for write, and did the whole refresh there (/data/appscan, APK signature read,packages.list(.tmp)read). A lookup in/data/systemthat misses the dcache takes the samei_rwseminlookup_slow()and the task deadlocks against itself.In 0.13.3/0.13.4 the
packages.xmlfallback did this and frozesystem_serveron a Pixel 8 Pro during Play Store batch updates (bmax121/APatch#1371):a9224bfremoved thepackages.xmlread, butmainstill does all the other I/O under the lock; it only works because thepackages.list.tmpdentry happens to be cached.Changes
task_work_add(current, ..., TWA_RESUME); the refresh runs on return to user space, after all VFS locks are dropped, in the same task. It reads the finalpackages.list(use_tmp=0).task_work_addcan't be resolved the rename hook is not installed; if it fails at runtime the refresh is skipped and logged.uid_listenerandreload-cfgstill refresh from process context./data/appscan by renaming its own.../system/packages.list.tmp.Testing
Same trigger on each build: drop caches, then rename
packages.list.tmp->packages.listas uid 1000.main+ oldpackages.xmlfallbackmvin D state, stack abovepackages.xmlread still presentpm installwith cold cacherc=0(manager trusted in the same event)hook cfi_handler rc: 0,bypass_kcfi done: 0; deferred callback runs under kCFI, no panicaarch64-none-elf-gcc12.2): https://github.com/mountaser/KernelPatch/actions/runs/37575168663selinux_inode_renamehook goes through the samerefresh_packages_list_tmp_rename(), so it gets the deferral and uid filter too).Not included
lkm/manager/manager.chas the same synchronouskp_manager_scan(1)in its rename hook; a correct port also has to handle unload with a work item queued.filp_openinsideiterate_dirduring the/data/appscan, addressed in a separate PR (android: never look up children while iterating /data/app #327).