Repository navigation
Conversation
On kernels >= 6.1 the trusted-manager APK scan descended into each /data/app/~~<hash> directory from inside the /data/app iterate_dir() actor (apk_outer_actor -> filp_open). iterate_dir() holds the /data/app i_rwsem shared. A child lookup that misses the dcache takes it again in lookup_slow(). If a writer is already queued, the nested down_read() blocks behind it and the task deadlocks against itself. Every later access to /data/app then piles up. Seen on a Pixel 8 Pro (6.1.162 GKI) under a cold-dcache pm install loop. PackageManager sat in D state in down_write <- filename_create <- do_mkdirat. apd's refresh sat in D state in down_read <- path_openat <- filp_open <- [KP] <- f2fs_readdir <- iterate_dir. This is the supercall refresh path, so deferring the rename-hook refresh does not cover it. Use a two-phase scan on all kernels. Phase 1 only records "~~" names while iterating, with no lookup, I/O or allocation in the actor. Phase 2 opens and scans the recorded dirs, in iteration order, after iterate_dir() has returned. 06887a4 limited the two-phase scan to old kernels because the collector could miss the APK: it kept at most 256 names of fewer than 64 bytes and silently dropped the rest. The new name list is packed and growable. When it fills up, the actor stops and flags an overflow. The caller doubles the buffer with the lock released and rescans from the start. So no name is ever dropped. Past 2 MiB of names the scan fails loudly with -E2BIG instead of truncating. The same list replaces the fixed array on old kernels, which had the same silent cap. The flat pass, log lines and return codes are unchanged. The first matching ~~ dir in iteration order still wins.
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.
Summary
Never look up children while
iterate_dir()holds/data/app, on any kernel version: the/data/appAPK scan is now two-phase everywhere.Problem
On kernels >= 6.1,
apk_outer_actorcalledfilp_open("/data/app/~~<hash>/")from inside theiterate_dir()actor.iterate_dir()holds the/data/appi_rwsemshared, and a child lookup that misses the dcache takes it again inlookup_slow(). If a writer is already queued (an install doingmkdir), the seconddown_read()blocks behind it and the task deadlocks against itself; everything touching/data/appthen piles up.Seen on a Pixel 8 Pro (6.1.162 GKI) with a cold-dcache
pm installloop:This is the
uid_listener/ supercall refresh path, so it is independent of the rename hook (#326).Changes
iterate_dir()actor only records~~names (no lookup, I/O or allocation).iterate_dir()returns and the lock is released, open and scan each recorded directory in iteration order.06887a4limited the two-phase scan to old kernels because the collector could miss the APK: it kept at most 256 names of < 64 bytes and silently dropped the rest. The new list is packed and growable: when it fills, the actor sets an overflow flag, the caller doubles the buffer with no lock held and rescans from the start. Past 2 MiB of names the scan fails loudly with-E2BIGinstead of truncating. The same list replaces the fixed array on old kernels.~~directory in iteration order still wins.Testing
apk found (scramble)), including 54uid_listenerevents (theapdrefresh path); 0 hangs, 0app dir scanerrors~~dirs (list grew 16 -> 32 -> 64 KiB) + 20 cold-cache installsrc=0, 0 scan errorsaarch64-none-elf-gcc12.2): https://github.com/mountaser/KernelPatch/actions/runs/37581922702 (this change + android: defer packages.list rename refresh out of the rename lock #326). This branch alone compiles with 0 errors under clang; it merges cleanly with android: defer packages.list rename refresh out of the rename lock #326 and android: hook selinux_inode_rename when the rename LSM wrapper is inlined #322.