Zygisk: mirror the kernel's answer for an absent interface - #311
Merged
Conversation
@sliva_ru reported `setsockopt SO_BINDTODEVICE tun0` leaking on a self-built LineageOS sm8350 (5.4.302-qgki) while the KPM was fully healthy — every hook installed, error 0x0. The vector was never covered there: the resolved-ifindex hook was wired only for kernels in [5.7, 5.9), and below that we deliberately relied on the kernel refusing an unprivileged bind without CAP_NET_RAW. That assumption came from upstream sources and the QEMU reference images, and it does not survive contact with vendor trees. His kernel has the gate compiled in — `sock_bindtoindex_locked` calls `ns_capable`, verified by disassembling the vmlinux rebuilt from his boot image — and an untrusted app still bound a socket to tun0. Whatever satisfies the check on that ROM, a kernel-side policy we do not control cannot back a coverage claim. - probe both helper names by symbol on any kernel below 5.9: upstream renamed sock_setbindtodevice_locked to sock_bindtoindex_locked in 5.8, and 5.4 vendor trees backport the newer one (his does) - when neither resolves, leave the hook bit clear rather than reporting a vector we do not cover - the partial-hooks warning now fires only when a missing hook costs a measured vector, so kernels that never had the symbol — and close the surface by capability or SELinux anyway — stay quiet - correct the invariant in detection-vectors.md and at the guard in socket_bind_before_common, with the counterexample, so it does not get reintroduced. The zygisk hook still carries the same version gate; its oracle argument holds only while every bind is refused, noted in the doc as wanting a runtime probe.
Two devices report the app dying at startup whenever a VPN interface is up, with nothing in the app's own logs. The probe crate built with `panic = "abort"`, and the full check suite runs on every cold start, so any panic in a probe — parsing whatever an arbitrary vendor kernel hands back — took the process down with SIGABRT and no Java trace. The JNI entry already wrapped the run in `catch_unwind` (via jni's `with_env` + ThrowRuntimeExAndDefault); abort made that dead code. - release and dev profiles unwind. Costs ~39 KB of unwind tables on arm64, the only ABI we ship - a panic hook logs the message and its file:line to logcat under VpnHide-Native, which the bundle's filter now captures. The default hook writes to stderr, which for an app process goes nowhere — this is why such a crash leaves no trace. Backtraces are deliberately not attempted: fat LTO + strip would make them unsymbolised addresses - the Kotlin caller swallows a thrown probe run, so the worst case is one empty check run instead of a dead app - pass the SIOCGIFFLAGS/SIOCGIFMTU ifreq by unique reference. The kernel writes the result through that pointer, so deriving it from a shared borrow is UB — and it is the tun0-visible branch that reads the value back, exactly the state these reports are about
…d path The SIOCGIF* pre-screen dereferenced the caller's `arg` before the real ioctl had a chance to validate it. A target app passing a bad or short pointer — the case the kernel answers with EFAULT — instead took a SIGSEGV inside its own process, caused by our hook. The setsockopt hook was written to avoid exactly this (copy_from_self, with the reasoning in its doc comment); the ioctl path was the inconsistent one. Reads the 16-byte name through copy_from_self and passes a non-socket fd straight through, since this ioctl family cannot apply to one. Fault containment itself is already covered by self_copy_contains_bad_caller_pointers. Found by a review pass over the project's unsafe code.
`writeSuperkeySetting` and `writeFilesystemHidingSetting` each change one field in `settings`, but took their base from `buildCanonicalConfigFromTargetsSnapshot` — rebuilding every app's roles from the snapshot's per-role sets and writing that back. One of those sets round-tripped through UIDs: `appHiding` was stored as resolved UIDs and mapped back through `pm list packages`, so a target the inventory could not see (a profile the scan failed to read — the case #293 added diagnostics for) disappeared from the projection and lost its role on disk. A toggle unrelated to the app list silently unconfigured an app. - both writers take `snapshot.canonicalConfig` and copy the one field - TargetsSnapshot no longer stores the five per-role sets beside the config they came from. They are projections of it now, so the object cannot carry two versions of one truth, and `observerNames` no longer passes through UIDs at all. `observerUids` stays for the consumers that need the wire's language, derived on demand - regression test: an app-hiding target absent from the inventory keeps its role Not reproduced on a device — the chain is read off the code, and the missing-from-inventory precondition is one users have hit.
The index-helper hook runs before that helper's own CAP_NET_RAW check, so it answered ENODEV even for callers the kernel was about to refuse with EPERM. On a tree where the check bites, a VPN name then reads differently from every other name — the exact oracle the Zygisk hook refuses to create — and widening the hook to all kernels below 5.9 in the previous commit widened that to the 4.x families where the check really does bite. Ask capable(CAP_NET_RAW) first and stay out of the way when it fails: the kernel refuses those callers itself, identically for every interface. Deny only the ones that would otherwise have bound, which is the case the LineageOS 5.4 report is about. capable() needs no struct offsets, so it cannot go stale on a vendor kernel the way an offset table can; a kernel where it will not resolve keeps the previous unconditional denial.
The bind hook decided whether to act by comparing the release string: below 5.7 it stayed inert, on the assumption that the kernel refuses an unprivileged bind before it even parses the name, so a name-specific ENODEV from us would announce the interface instead of hiding it. The oracle argument is sound; the version test is not the way to ask it. A LineageOS 5.4 build (the KPM report in the previous commit) lets an app bind to tun0, and there the same gate means we simply do not hide. Replaced with the question that actually decides it: what does this kernel return for a bind to a name that cannot exist? Denying a hidden interface with exactly that errno is oracle-free by construction — EPERM where every bind is refused (indistinguishable, as today), ENODEV where names resolve first (hidden, as on 5.7+). - one socket + one setsockopt through the real libc entry, cached for the process; an unusable measurement falls back to the old heuristic, so behaviour is never worse than before - hidden_bind_errno is pure and unit-tested over the decision table, and the hook test now covers the EPERM-mirroring path end to end - bind-probe gains a bind_absent_name case, so the QEMU lanes record what each supported kernel family answers instead of us assuming it
okhsunrog
force-pushed
the
fix/zygisk-bind-probe
branch
from
August 25, 2026 09:57
13e6139 to
ba6311e
Compare
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
1 participant
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.
Stacked on #310 — review that first; this adds one commit on top.
The zygisk bind hook decided whether to act by comparing the release string: below 5.7 it stayed inert, on the assumption that the kernel refuses an unprivileged bind before it even parses the name, so a name-specific ENODEV from us would announce the interface instead of hiding it. The oracle argument is sound; the version test is not the way to ask it. The LineageOS 5.4 build behind #310 lets an app bind to tun0, and there the same gate means we simply do not hide.
Replaced with the question that actually decides it: what does this kernel return for a bind to a name that cannot exist? Denying a hidden interface with exactly that errno is oracle-free by construction — EPERM where every bind is refused (indistinguishable, as today), ENODEV where names resolve first (hidden, as on 5.7+).
hidden_bind_errnois pure and unit-tested over the decision table, and the existing hook test now also covers the EPERM-mirroring path end to endbind-probegains abind_absent_namecase, so the QEMU lanes record what each supported kernel family actually answers (4.9 / 4.14 / 4.19 / 5.4 through kpm-qemu-legacy, GKI through kmod-qemu) instead of the code assuming itNo device testing was needed for this: the kernel-side question is measured by the existing QEMU rig, and the hook's decision is a pure function with host tests. The thin probe is two syscalls whose failure mode is the old behaviour.