diff --git a/kmod/vpnhide_kmod.c b/kmod/vpnhide_kmod.c index 894f88d..2456f81 100644 --- a/kmod/vpnhide_kmod.c +++ b/kmod/vpnhide_kmod.c @@ -76,20 +76,18 @@ static DEFINE_SPINLOCK(uids_lock); static bool is_target_uid(void) { uid_t uid = from_kuid(&init_user_ns, current_uid()); + bool found = false; int i; - if (READ_ONCE(nr_target_uids) == 0) - return false; - spin_lock(&uids_lock); for (i = 0; i < nr_target_uids; i++) { if (target_uids[i] == uid) { - spin_unlock(&uids_lock); - return true; + found = true; + break; } } spin_unlock(&uids_lock); - return false; + return found; } /* ------------------------------------------------------------------ */ @@ -284,13 +282,13 @@ static int dev_ifconf_ret(struct kretprobe_instance *ri, for (i = 0; i < n; i++) { if (copy_from_user(&tmp, &usr_ifr[i], sizeof(tmp))) - break; + return 0; /* copy failed — leave buffer untouched */ tmp.ifr_name[IFNAMSIZ - 1] = '\0'; if (is_vpn_ifname(tmp.ifr_name)) continue; if (dst != i) { if (copy_to_user(&usr_ifr[dst], &tmp, sizeof(tmp))) - break; + return 0; /* copy failed — stop compacting */ } dst++; } @@ -343,8 +341,12 @@ static int rtnl_fill_entry(struct kretprobe_instance *ri, * arm64: x0=skb, x1=dev */ dev = (struct net_device *)regs->regs[1]; + /* Callers hold RTNL which protects dev->name, but take RCU as + * belt-and-suspenders — same rationale as inet6_fill_entry. */ + rcu_read_lock(); if (dev && is_vpn_ifname(dev->name)) data->should_filter = true; + rcu_read_unlock(); return 0; } @@ -403,12 +405,19 @@ static int inet6_fill_entry(struct kretprobe_instance *ri, return 0; ifa = (struct inet6_ifaddr *)regs->regs[1]; + /* + * The callers of inet6_fill_ifaddr() hold either rcu_read_lock() + * (netlink dump path) or RTNL. We take rcu_read_lock() explicitly + * so the kretprobe handler doesn't rely on that implicit guarantee. + */ + rcu_read_lock(); if (ifa && ifa->idev && ifa->idev->dev && is_vpn_ifname(ifa->idev->dev->name)) { data->skb = (struct sk_buff *)regs->regs[0]; data->saved_len = data->skb ? data->skb->len : 0; data->should_filter = true; } + rcu_read_unlock(); return 0; } @@ -462,12 +471,15 @@ static int inet_fill_entry(struct kretprobe_instance *ri, return 0; ifa = (struct in_ifaddr *)regs->regs[1]; + /* Same RCU rationale as inet6_fill_entry above. */ + rcu_read_lock(); if (ifa && ifa->ifa_dev && ifa->ifa_dev->dev && is_vpn_ifname(ifa->ifa_dev->dev->name)) { data->skb = (struct sk_buff *)regs->regs[0]; data->saved_len = data->skb ? data->skb->len : 0; data->should_filter = true; } + rcu_read_unlock(); return 0; } @@ -617,7 +629,7 @@ static struct kretprobe_reg probes[] = { static int __init vpnhide_init(void) { - int i, ret; + int i, ret, ok = 0; for (i = 0; i < ARRAY_SIZE(probes); i++) { ret = register_kretprobe(probes[i].krp); @@ -626,11 +638,21 @@ static int __init vpnhide_init(void) probes[i].name, ret); } else { probes[i].registered = true; + ok++; pr_info(MODNAME ": kretprobe(%s) registered\n", probes[i].name); } } + if (ok == 0) { + pr_err(MODNAME ": no kretprobes registered, aborting\n"); + return -ENOENT; + } + if (ok < ARRAY_SIZE(probes)) + pr_warn(MODNAME ": only %d/%zu kretprobes registered — " + "some detection paths are not covered\n", + ok, ARRAY_SIZE(probes)); + /* 0600: root-only read/write. UIDs are written here by service.sh * and WebUI (both root). Apps must not see the target list. */ targets_entry = proc_create("vpnhide_targets", 0600, NULL, diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/HookEntry.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/HookEntry.kt index 37d556b..d14f419 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/HookEntry.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/HookEntry.kt @@ -9,10 +9,8 @@ import de.robv.android.xposed.XC_MethodHook import de.robv.android.xposed.XposedBridge import de.robv.android.xposed.XposedHelpers import de.robv.android.xposed.callbacks.XC_LoadPackage -import java.io.BufferedReader import java.io.File -import java.io.FileInputStream -import java.io.InputStreamReader +import java.util.concurrent.atomic.AtomicBoolean /** * VpnHide — hide VPN presence from apps via system_server Binder hooks. @@ -37,25 +35,24 @@ import java.io.InputStreamReader */ class HookEntry : IXposedHookLoadPackage { + private val hookInstalled = AtomicBoolean(false) + override fun handleLoadPackage(lpparam: XC_LoadPackage.LoadPackageParam) { // Only hook system_server. handleLoadPackage fires multiple times - // in system_server (once per hosted package / APEX), so we use a - // flag to install hooks exactly once. - val inSystemServer = isSystemServer || + // in system_server (once per hosted package / APEX), so we use + // compareAndSet to install hooks exactly once. + val inSystemServer = hookInstalled.get() || lpparam.processName == "android" || android.os.Process.myUid() == 1000 if (!inSystemServer) return - if (!isSystemServer) { - isSystemServer = true + if (hookInstalled.compareAndSet(false, true)) { XposedBridge.log("VpnHide: system_server detected, installing Binder hooks") installSystemServerHooks() } } - @Volatile private var isSystemServer = false - private inline fun tryHook(name: String, block: () -> Unit) { try { block() @@ -89,31 +86,40 @@ class HookEntry : IXposedHookLoadPackage { @Volatile private var systemServerTargetUids: Set? = null @Volatile private var targetUidsFileObserver: android.os.FileObserver? = null + private val uidLock = Any() private fun loadTargetUids(): Set { + // Fast path: already cached (volatile read) systemServerTargetUids?.let { return it } - val uids = mutableSetOf() + // Slow path: only one thread reads the file + synchronized(uidLock) { + systemServerTargetUids?.let { return it } - // Read pre-resolved numeric UIDs written by vpnhide-kmod's - // service.sh into /data/system/vpnhide_uids.txt. - // system_server can read /data/system/ (SELinux: system_data_file). - try { - val file = File("/data/system/vpnhide_uids.txt") - if (file.exists()) { - file.readLines().forEach { line -> - line.trim().toIntOrNull()?.let { uids.add(it) } + val uids = mutableSetOf() + + // Read pre-resolved numeric UIDs written by vpnhide-kmod's + // service.sh into /data/system/vpnhide_uids.txt. + // system_server can read /data/system/ (SELinux: system_data_file). + try { + val file = File("/data/system/vpnhide_uids.txt") + if (file.exists()) { + file.readLines().forEach { line -> + line.trim().toIntOrNull()?.let { uids.add(it) } + } } + } catch (t: Throwable) { + XposedBridge.log("VpnHide: failed to read UIDs: ${t.message}") } - } catch (t: Throwable) { - XposedBridge.log("VpnHide: failed to read UIDs: ${t.message}") - } - if (uids.isNotEmpty()) { - XposedBridge.log("VpnHide: system_server loaded ${uids.size} target UIDs: $uids") - systemServerTargetUids = uids + val result: Set = uids.toSet() + if (result.isNotEmpty()) { + XposedBridge.log("VpnHide: system_server loaded ${result.size} target UIDs: $result") + } + // Always cache (even if empty) to avoid re-reading until invalidated + systemServerTargetUids = result + return result } - return uids } private fun isTargetCaller(): Boolean { @@ -180,17 +186,29 @@ class HookEntry : IXposedHookLoadPackage { val vpnBit = 1L shl TRANSPORT_VPN if (transportTypes and vpnBit == 0L) return - savedTransport.set(transportTypes) + // Read all original values before mutating anything val caps = XposedHelpers.getLongField(nc, "mNetworkCapabilities") - savedCaps.set(caps) val ti = try { XposedHelpers.getObjectField(nc, "mTransportInfo") } catch (_: Throwable) { null } - savedTi.set(ti) - XposedHelpers.setLongField(nc, "mTransportTypes", transportTypes and vpnBit.inv()) - XposedHelpers.setLongField(nc, "mNetworkCapabilities", caps or (1L shl NET_CAPABILITY_NOT_VPN)) - if (ti != null && ti.javaClass.name == "android.net.VpnTransportInfo") { - XposedHelpers.setObjectField(nc, "mTransportInfo", null) + // Mutate — if any step fails, restore everything + try { + XposedHelpers.setLongField(nc, "mTransportTypes", transportTypes and vpnBit.inv()) + XposedHelpers.setLongField(nc, "mNetworkCapabilities", caps or (1L shl NET_CAPABILITY_NOT_VPN)) + if (ti != null && ti.javaClass.name == "android.net.VpnTransportInfo") { + XposedHelpers.setObjectField(nc, "mTransportInfo", null) + } + } catch (t: Throwable) { + // Restore on partial mutation failure + XposedHelpers.setLongField(nc, "mTransportTypes", transportTypes) + XposedHelpers.setLongField(nc, "mNetworkCapabilities", caps) + XposedHelpers.setObjectField(nc, "mTransportInfo", ti) + throw t } + + // Only save after all mutations succeeded + savedTransport.set(transportTypes) + savedCaps.set(caps) + savedTi.set(ti) } catch (t: Throwable) { XposedBridge.log("VpnHide: NC.writeToParcel before error: ${t.message}") } @@ -199,13 +217,15 @@ class HookEntry : IXposedHookLoadPackage { override fun afterHookedMethod(param: MethodHookParam) { val origTransport = savedTransport.get() ?: return savedTransport.remove() + val origCaps = savedCaps.get() + savedCaps.remove() + val origTi = savedTi.get() + savedTi.remove() val nc = param.thisObject try { XposedHelpers.setLongField(nc, "mTransportTypes", origTransport) - savedCaps.get()?.let { XposedHelpers.setLongField(nc, "mNetworkCapabilities", it) } - savedCaps.remove() - XposedHelpers.setObjectField(nc, "mTransportInfo", savedTi.get()) - savedTi.remove() + if (origCaps != null) XposedHelpers.setLongField(nc, "mNetworkCapabilities", origCaps) + XposedHelpers.setObjectField(nc, "mTransportInfo", origTi) } catch (_: Throwable) {} } } @@ -214,43 +234,78 @@ class HookEntry : IXposedHookLoadPackage { } /** - * Hook NetworkInfo.writeToParcel — skip VPN NetworkInfo for target callers. + * Hook NetworkInfo.writeToParcel — disguise VPN NetworkInfo for target callers. + * Instead of skipping writeToParcel (which would corrupt the Parcel stream + * since the non-null flag is already written by the AIDL stub), temporarily + * change the type to WIFI and let serialization proceed normally. */ private fun hookNIWriteToParcel() { XposedHelpers.findAndHookMethod( NetworkInfo::class.java, "writeToParcel", android.os.Parcel::class.java, Integer.TYPE, object : XC_MethodHook() { + private val savedType = ThreadLocal() + override fun beforeHookedMethod(param: MethodHookParam) { if (!isTargetCaller()) return - val info = param.thisObject as NetworkInfo - @Suppress("DEPRECATION") - if (info.type == TYPE_VPN) { - param.result = null + val ni = param.thisObject + try { + val type = XposedHelpers.getIntField(ni, "mNetworkType") + if (type != TYPE_VPN) return + + savedType.set(type) + XposedHelpers.setIntField(ni, "mNetworkType", TYPE_WIFI) + } catch (t: Throwable) { + XposedBridge.log("VpnHide: NI.writeToParcel before error: ${t.message}") } } + + override fun afterHookedMethod(param: MethodHookParam) { + val origType = savedType.get() ?: return + savedType.remove() + try { + XposedHelpers.setIntField(param.thisObject, "mNetworkType", origType) + } catch (_: Throwable) {} + } } ) XposedBridge.log("VpnHide: hooked NetworkInfo.writeToParcel") } /** - * Hook LinkProperties.writeToParcel — null out VPN-interface LPs - * for target callers. + * Hook LinkProperties.writeToParcel — clear VPN interface name from + * LP for target callers. Instead of skipping writeToParcel (which + * would corrupt the Parcel stream), temporarily clear the interface + * name and let serialization proceed normally. */ private fun hookLPWriteToParcel() { XposedHelpers.findAndHookMethod( LinkProperties::class.java, "writeToParcel", android.os.Parcel::class.java, Integer.TYPE, object : XC_MethodHook() { + private val savedIfname = ThreadLocal() + override fun beforeHookedMethod(param: MethodHookParam) { if (!isTargetCaller()) return - val lp = param.thisObject as LinkProperties - val ifname = lp.interfaceName ?: return - if (isVpnInterfaceName(ifname)) { - param.result = null + val lp = param.thisObject + try { + val ifname = XposedHelpers.getObjectField(lp, "mIfaceName") as? String ?: return + if (!isVpnInterfaceName(ifname)) return + + savedIfname.set(ifname) + XposedHelpers.setObjectField(lp, "mIfaceName", null) + } catch (t: Throwable) { + XposedBridge.log("VpnHide: LP.writeToParcel before error: ${t.message}") } } + + override fun afterHookedMethod(param: MethodHookParam) { + val origIfname = savedIfname.get() ?: return + savedIfname.remove() + try { + XposedHelpers.setObjectField(param.thisObject, "mIfaceName", origIfname) + } catch (_: Throwable) {} + } } ) XposedBridge.log("VpnHide: hooked LinkProperties.writeToParcel") @@ -260,5 +315,6 @@ class HookEntry : IXposedHookLoadPackage { private const val TRANSPORT_VPN = 4 private const val NET_CAPABILITY_NOT_VPN = 15 private const val TYPE_VPN = 17 + private const val TYPE_WIFI = 1 } } diff --git a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/MainActivity.kt b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/MainActivity.kt index 581d061..0e015d8 100644 --- a/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/MainActivity.kt +++ b/lsposed/app/src/main/kotlin/dev/okhsunrog/vpnhide/MainActivity.kt @@ -57,9 +57,18 @@ class MainActivity : ComponentActivity() { private fun suExec(cmd: String): Pair { return try { val proc = Runtime.getRuntime().exec(arrayOf("su", "-c", cmd)) - val stdout = proc.inputStream.bufferedReader().readText() - val exitCode = proc.waitFor() - exitCode to stdout + try { + // Drain stderr in the background to prevent the process from + // blocking if it produces error output. + val stderrDrain = Thread { proc.errorStream.readBytes() } + stderrDrain.start() + val stdout = proc.inputStream.bufferedReader().readText() + val exitCode = proc.waitFor() + stderrDrain.join() + exitCode to stdout + } finally { + proc.destroy() + } } catch (e: Exception) { Log.e(TAG, "su exec failed: ${e.message}") -1 to "" diff --git a/zygisk/src/filter.rs b/zygisk/src/filter.rs index 14cbd4f..7a8b3eb 100644 --- a/zygisk/src/filter.rs +++ b/zygisk/src/filter.rs @@ -349,12 +349,14 @@ const fn nlmsg_align(len: usize) -> usize { (len + NLMSG_ALIGNTO - 1) & !(NLMSG_ALIGNTO - 1) } -fn read_u32_ne(data: &[u8], off: usize) -> u32 { - u32::from_ne_bytes([data[off], data[off + 1], data[off + 2], data[off + 3]]) +fn read_u32_ne(data: &[u8], off: usize) -> Option { + let bytes: &[u8; 4] = data.get(off..off + 4)?.try_into().ok()?; + Some(u32::from_ne_bytes(*bytes)) } -fn read_u16_ne(data: &[u8], off: usize) -> u16 { - u16::from_ne_bytes([data[off], data[off + 1]]) +fn read_u16_ne(data: &[u8], off: usize) -> Option { + let bytes: &[u8; 2] = data.get(off..off + 2)?.try_into().ok()?; + Some(u16::from_ne_bytes(*bytes)) } /// Filter netlink dump responses in-place: remove `RTM_NEWLINK` and @@ -375,19 +377,21 @@ pub fn filter_netlink_dump(data: &mut [u8], vpn_indices: &[u32]) -> usize { let mut write_pos = 0usize; while read_pos + NLMSG_HDRLEN <= len { - let nlmsg_len = read_u32_ne(data, read_pos) as usize; + let Some(nlmsg_len_raw) = read_u32_ne(data, read_pos) else { break }; + let nlmsg_len = nlmsg_len_raw as usize; if nlmsg_len < NLMSG_HDRLEN || read_pos + nlmsg_len > len { break; } let aligned_len = nlmsg_align(nlmsg_len).min(len - read_pos); - let nlmsg_type = read_u16_ne(data, read_pos + 4); + let Some(nlmsg_type) = read_u16_ne(data, read_pos + 4) else { break }; let hide = if (nlmsg_type == RTM_NEWLINK || nlmsg_type == RTM_NEWADDR) && nlmsg_len >= NLMSG_HDRLEN + 8 { // Interface index is at payload offset 4 in both // ifinfomsg and ifaddrmsg. - let if_index = read_u32_ne(data, read_pos + NLMSG_HDRLEN + 4); + let if_index = read_u32_ne(data, read_pos + NLMSG_HDRLEN + 4) + .unwrap_or(0); vpn_indices.contains(&if_index) } else { false diff --git a/zygisk/src/hooks.rs b/zygisk/src/hooks.rs index 629ba2f..6b7f5b9 100644 --- a/zygisk/src/hooks.rs +++ b/zygisk/src/hooks.rs @@ -529,10 +529,23 @@ unsafe fn open_filtered_proc_net( } if filtered_len > 0 { - unsafe { - libc::write(memfd, buf.as_ptr() as *const c_void, filtered_len); - libc::lseek(memfd, 0, libc::SEEK_SET); + let mut written = 0usize; + while written < filtered_len { + let n = unsafe { + libc::write( + memfd, + buf[written..].as_ptr() as *const c_void, + filtered_len - written, + ) + }; + if n < 0 { + unsafe { libc::close(memfd) }; + set_errno(libc::EIO); + return -1; + } + written += n as usize; } + unsafe { libc::lseek(memfd, 0, libc::SEEK_SET) }; } memfd diff --git a/zygisk/src/shadowhook.rs b/zygisk/src/shadowhook.rs index 022a950..1bddf29 100644 --- a/zygisk/src/shadowhook.rs +++ b/zygisk/src/shadowhook.rs @@ -7,7 +7,7 @@ //! `aarch64-linux-android`; see `build.rs`. use core::ffi::{c_char, c_int, c_void}; -use core::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Once; #[allow(non_camel_case_types)] #[repr(C)] @@ -41,22 +41,22 @@ unsafe extern "C" { fn shadowhook_unhook(stub: *mut c_void) -> c_int; } -static INIT_DONE: AtomicBool = AtomicBool::new(false); +static INIT: Once = Once::new(); +static mut INIT_RC: c_int = 0; /// Initialize shadowhook exactly once per process. Returns Ok on success /// or if already initialized; Err with the raw return code otherwise. pub fn init_once() -> Result<(), c_int> { - if INIT_DONE.load(Ordering::Acquire) { - return Ok(()); - } - // SAFETY: FFI call with no arguments that reference Rust memory. - let rc = unsafe { shadowhook_init(ShadowhookMode::Unique as c_int, false) }; - if rc == 0 { - INIT_DONE.store(true, Ordering::Release); - Ok(()) - } else { - Err(rc) - } + INIT.call_once(|| { + // SAFETY: FFI call with no arguments that reference Rust memory. + let rc = unsafe { shadowhook_init(ShadowhookMode::Unique as c_int, false) }; + // SAFETY: written exactly once inside call_once, read only after. + unsafe { INIT_RC = rc }; + }); + // SAFETY: INIT_RC is only written inside call_once above and never + // mutated again; all reads happen after call_once has completed. + let rc = unsafe { INIT_RC }; + if rc == 0 { Ok(()) } else { Err(rc) } } /// Install an inline hook on `lib!sym`. `new_fn` is the replacement