From 56cde3c2e396d9759fc9fe45cec0e9e816db94d0 Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 26 Feb 2025 18:06:30 +0100 Subject: [PATCH] Use socket instead of ping command when pinging on android MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous implementation spawned a process with tokio which in turn registered a signal handler without ONASTACK flag set. When using GO code, all signal handlers needs to have this flag set otherwise a signal might be handled on a goroutine thread which has a small stack and thus can overflow. Reference: DROID-1825 Co-authored-by: David Lönnhager --- Cargo.lock | 1 - talpid-wireguard/Cargo.toml | 5 -- .../src/connectivity/pinger/android.rs | 72 ------------------- .../src/connectivity/pinger/icmp.rs | 14 +++- .../src/connectivity/pinger/mod.rs | 12 +--- 5 files changed, 14 insertions(+), 90 deletions(-) delete mode 100644 talpid-wireguard/src/connectivity/pinger/android.rs diff --git a/Cargo.lock b/Cargo.lock index aa087cf1265f..1547eb135307 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4783,7 +4783,6 @@ dependencies = [ "bitflags 1.3.2", "byteorder", "chrono", - "duct", "futures", "hex", "internet-checksum", diff --git a/talpid-wireguard/Cargo.toml b/talpid-wireguard/Cargo.toml index bce2fc2090ce..9cac2c2970c6 100644 --- a/talpid-wireguard/Cargo.toml +++ b/talpid-wireguard/Cargo.toml @@ -31,11 +31,6 @@ rand = "0.8.5" surge-ping = "0.8.0" rand_chacha = "0.3.1" wireguard-go-rs = { path = "../wireguard-go-rs"} - -[target.'cfg(target_os="android")'.dependencies] -duct = "0.13" - -[target.'cfg(not(target_os="android"))'.dependencies] byteorder = "1" internet-checksum = "0.2" socket2 = { workspace = true, features = ["all"] } diff --git a/talpid-wireguard/src/connectivity/pinger/android.rs b/talpid-wireguard/src/connectivity/pinger/android.rs deleted file mode 100644 index 34e28f8891f9..000000000000 --- a/talpid-wireguard/src/connectivity/pinger/android.rs +++ /dev/null @@ -1,72 +0,0 @@ -use std::net::Ipv4Addr; -use std::process::Stdio; -use std::time::Duration; - -use tokio::io; -use tokio::process::{Child, Command}; - -/// Pinger errors -#[derive(thiserror::Error, Debug)] -pub enum Error { - /// Failed to run `ping` process - #[error("Failed to run ping command")] - PingError(#[from] io::Error), - - /// ICMP timed out - #[error("Ping timed out")] - TimeoutError, -} - -/// A pinger that sends ICMP requests without waiting for responses -pub struct Pinger { - addr: Ipv4Addr, - processes: Vec, -} - -impl Pinger { - /// Creates a new pinger that will send ICMP requests only through the specified interface - pub fn new(addr: Ipv4Addr) -> Result { - Ok(Self { - processes: vec![], - addr, - }) - } - - fn try_deplete_process_list(&mut self) { - self.processes.retain_mut(|child| { - // retain non-terminated children - matches!(child.try_wait(), Err(_) | Ok(None)) - }); - } -} - -#[async_trait::async_trait] -impl super::Pinger for Pinger { - // Send an ICMP packet without waiting for a reply - async fn send_icmp(&mut self) -> Result<(), Error> { - self.try_deplete_process_list(); - - let child = ping_cmd(self.addr, Duration::from_secs(1)).map_err(Error::PingError)?; - self.processes.push(child); - Ok(()) - } - - async fn reset(&mut self) { - self.processes.clear(); - } -} - -fn ping_cmd(ip: Ipv4Addr, timeout: Duration) -> io::Result { - let mut cmd = Command::new("ping"); - - let timeout_secs = timeout.as_secs().to_string(); - let ip = ip.to_string(); - cmd.args(["-n", "-i", "1", "-w", &timeout_secs, &ip]); - - cmd.stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .kill_on_drop(true); - - cmd.spawn() -} diff --git a/talpid-wireguard/src/connectivity/pinger/icmp.rs b/talpid-wireguard/src/connectivity/pinger/icmp.rs index b17ee7ddc1b7..338f364844d1 100644 --- a/talpid-wireguard/src/connectivity/pinger/icmp.rs +++ b/talpid-wireguard/src/connectivity/pinger/icmp.rs @@ -62,11 +62,19 @@ impl Pinger { /// Creates a new `Pinger`. pub fn new( addr: Ipv4Addr, - #[cfg(not(target_os = "windows"))] interface_name: String, + #[cfg(any(target_os = "linux", target_os = "macos"))] interface_name: String, ) -> Result { let addr = SocketAddr::new(addr.into(), 0); - let sock = - Socket::new(Domain::IPV4, Type::RAW, Some(Protocol::ICMPV4)).map_err(Error::Open)?; + let sock = Socket::new( + Domain::IPV4, + if cfg!(target_os = "android") { + Type::DGRAM + } else { + Type::RAW + }, + Some(Protocol::ICMPV4), + ) + .map_err(Error::Open)?; sock.set_nonblocking(true).map_err(Error::Open)?; #[cfg(target_os = "linux")] diff --git a/talpid-wireguard/src/connectivity/pinger/mod.rs b/talpid-wireguard/src/connectivity/pinger/mod.rs index 10875afb8af7..b6e459429f9d 100644 --- a/talpid-wireguard/src/connectivity/pinger/mod.rs +++ b/talpid-wireguard/src/connectivity/pinger/mod.rs @@ -1,12 +1,6 @@ -#[cfg(target_os = "android")] -#[path = "android.rs"] -mod imp; +mod icmp; -#[cfg(any(target_os = "windows", target_os = "linux", target_os = "macos"))] -#[path = "icmp.rs"] -mod imp; - -pub use imp::Error; +pub use icmp::Error; /// Trait for sending ICMP requests to get some traffic from a remote server #[async_trait::async_trait] @@ -22,7 +16,7 @@ pub fn new_pinger( addr: std::net::Ipv4Addr, #[cfg(any(target_os = "linux", target_os = "macos"))] interface_name: String, ) -> Result, Error> { - Ok(Box::new(imp::Pinger::new( + Ok(Box::new(icmp::Pinger::new( addr, #[cfg(any(target_os = "linux", target_os = "macos"))] interface_name,