From 8ed8fdbc521da3ef3de4c80f637a7395fba9c88f Mon Sep 17 00:00:00 2001 From: Nils Jeisecke Date: Wed, 17 Apr 2024 09:23:00 +0200 Subject: [PATCH] windows: fix memory leak The buffer must finally be released, so use a simple RAAI wrapper to handle this. Also limit retry to one reallocation (in case the initial 15K is not enough). --- Cargo.toml | 2 +- src/target/windows.rs | 227 +++++++++++++++++++++++------------------- 2 files changed, 124 insertions(+), 105 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 138da7a..3b03b92 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ cc = "1.0.73" [target.'cfg(target_os = "windows")'.dependencies] libc = "0.2.101" -winapi = {version = "0.3", features = ["ws2def", "ws2ipdef", "netioapi", "iphlpapi", "iptypes", "ntdef"] } +winapi = {version = "0.3", features = ["ws2def", "ws2ipdef", "netioapi", "iphlpapi", "iptypes", "ntdef", "winerror"] } [features] diff --git a/src/target/windows.rs b/src/target/windows.rs index da7dfcd..2fe5339 100644 --- a/src/target/windows.rs +++ b/src/target/windows.rs @@ -13,6 +13,7 @@ use winapi::{ netioapi::{ConvertLengthToIpv4Mask, ConvertInterfaceLuidToIndex}, ntdef::ULONG, ifdef::IF_LUID, + winerror, }, um::{ iptypes::{IP_ADAPTER_ADDRESSES_LH, IP_ADAPTER_UNICAST_ADDRESS_LH}, @@ -27,20 +28,6 @@ use crate::interface::Netmask; /// An alias for `IP_ADAPTER_ADDRESSES_LH` type AdapterAddress = IP_ADAPTER_ADDRESSES_LH; -/// The buffer size indicated by the `SizePointer` parameter is too small to hold the -/// adapter information or the `AdapterAddresses` parameter is `NULL`. The `SizePointer` -/// parameter returned points to the required size of the buffer to hold the adapter -/// information. -/// -/// Source: https://docs.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses#return-value -const ERROR_BUFFER_OVERFLOW: u32 = 111; - -/// Max tries allowed to call `GetAdaptersAddresses` on a loop basis -const MAX_TRIES: usize = 3; - -/// Success execution output from `GetAdaptersAddresses` call -const GET_ADAPTERS_ADDRESSES_SUCCESS_RESULT: u32 = 0; - /// A constant to store `winapi::shared::ws2def::AF_INET` casted as `u16` const AF_INET: u16 = winapi::shared::ws2def::AF_INET as u16; @@ -56,114 +43,146 @@ const GET_ADAPTERS_ADDRESSES_FAMILY: u32 = AF_UNSPEC as u32; /// A constant to store `winapi::um::iptypes::GAA_FLAG_INCLUDE_PREFIX` const GET_ADAPTERS_ADDRESSES_FLAGS: ULONG = winapi::um::iptypes::GAA_FLAG_INCLUDE_PREFIX; -impl NetworkInterfaceConfig for NetworkInterface { - fn show() -> Result> { - // Allocate a 15 KB buffer to start with. - let mut size_pointer: u32 = 15000; - let mut adapter_address = unsafe { malloc(size_pointer as usize) as *mut AdapterAddress }; - let mut iterations = 0; - let mut get_adapter_addresses_result = 0; - let mut network_interfaces: Vec = Vec::new(); +/// Safe memory management for the adapter address buffer +struct AdapterAddressBufferGuard { + p: *mut AdapterAddress, +} - while get_adapter_addresses_result != ERROR_BUFFER_OVERFLOW || iterations <= MAX_TRIES { - adapter_address = unsafe { malloc(size_pointer as usize) as *mut AdapterAddress }; +impl AdapterAddressBufferGuard { + fn new(buffer_size: usize) -> Option { + let p = unsafe { malloc(buffer_size) as *mut AdapterAddress }; + if p.is_null() { + None + } else { + Some(Self { p }) + } + } +} - if adapter_address.is_null() { - // Memory allocation failed for IP_ADAPTER_ADDRESSES struct - return Err(Error::GetIfAddrsError( - String::from("GetAdaptersAddresses"), - 1, - )); - } +impl Drop for AdapterAddressBufferGuard { + fn drop(&mut self) { + unsafe { + free(self.p as *mut c_void); + }; + } +} - get_adapter_addresses_result = unsafe { +impl NetworkInterfaceConfig for NetworkInterface { + fn show() -> Result> { + // Allocate a 15 KB buffer to start with. + let mut buffer_size: u32 = 15000; + // Retry once + let mut reallocated = false; + + let adapter_address = loop { + let adapter_address = + AdapterAddressBufferGuard::new(buffer_size as usize).ok_or_else(|| { + // Memory allocation failed for IP_ADAPTER_ADDRESSES struct + Error::GetIfAddrsError(String::from("GetAdaptersAddresses"), 1) + })?; + + let res = unsafe { GetAdaptersAddresses( GET_ADAPTERS_ADDRESSES_FAMILY, GET_ADAPTERS_ADDRESSES_FLAGS, null_mut(), - adapter_address, - &mut size_pointer, + adapter_address.p, + &mut buffer_size, ) }; - - if get_adapter_addresses_result == ERROR_BUFFER_OVERFLOW { - unsafe { - free(adapter_address as *mut c_void); - }; - adapter_address = null_mut(); - } else { - break; + match res { + winerror::ERROR_SUCCESS => { + break Ok(adapter_address); + } + winerror::ERROR_BUFFER_OVERFLOW => { + // The buffer size indicated by the `SizePointer` parameter is too small to hold the + // adapter information or the `AdapterAddresses` parameter is `NULL`. The `SizePointer` + // parameter returned points to the required size of the buffer to hold the adapter + // information. + // + // Source: https://docs.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses#return-value + if reallocated { + break Err(Error::GetIfAddrsError( + "GetAdapterAddresses: alloc error".to_string(), + res as i32, + )); + } + reallocated = true; + } + _ => { + break Err(Error::GetIfAddrsError( + "GetAdapterAddresses".to_string(), + res as i32, + )); + } } + }?; - iterations += 1; - } + // iterate over the contained structs + let mut adapter_address_it = adapter_address.p; + let mut network_interfaces: Vec = Vec::new(); - if get_adapter_addresses_result == GET_ADAPTERS_ADDRESSES_SUCCESS_RESULT { - while !adapter_address.is_null() { - let address_name = make_adapter_address_name(&adapter_address)?; - - let index = get_adapter_address_index(&adapter_address)?; - - // see https://docs.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses#examples - let mac_addr_len = unsafe { (*adapter_address).PhysicalAddressLength } as _; - let mac_addr = match mac_addr_len { - 0 => None, - len => Some(format!( - "{}", - HexSlice::new(unsafe { &(*adapter_address).PhysicalAddress[..len] }) - )), - }; - - // Find interface addresses - let mut current_unicast_address = unsafe { (*adapter_address).FirstUnicastAddress }; - - while !current_unicast_address.is_null() { - let address = unsafe { (*current_unicast_address).Address }; - - match unsafe { (*address.lpSockaddr).sa_family } { - AF_INET => { - let sockaddr: *mut SOCKADDR_IN = address.lpSockaddr as *mut SOCKADDR_IN; - let addr = make_ipv4_addr(&sockaddr); - let netmask = make_ipv4_netmask(¤t_unicast_address); - let network_interface = NetworkInterface::new_afinet( - &address_name, - addr, - netmask, - lookup_ipv4_broadcast_addr(&adapter_address, &sockaddr), - index, - ) - .with_mac_addr(mac_addr.clone()); - - network_interfaces.push(network_interface); - } - AF_INET6 => { - let sockaddr: *mut SOCKADDR_IN6 = - address.lpSockaddr as *mut SOCKADDR_IN6; - let addr = make_ipv6_addr(&sockaddr)?; - let netmask = make_ipv6_netmask(&sockaddr); - let network_interface = NetworkInterface::new_afinet6( - &address_name, - addr, - netmask, - None, - index, - ) - .with_mac_addr(mac_addr.clone()); - - network_interfaces.push(network_interface); - } - _ => {} - } + while !adapter_address_it.is_null() { + let address_name = make_adapter_address_name(&adapter_address_it)?; + + let index = get_adapter_address_index(&adapter_address_it)?; - if !current_unicast_address.is_null() { - current_unicast_address = unsafe { (*current_unicast_address).Next }; + // see https://docs.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses#examples + let mac_addr_len = unsafe { (*adapter_address_it).PhysicalAddressLength } as _; + let mac_addr = match mac_addr_len { + 0 => None, + len => Some(format!( + "{}", + HexSlice::new(unsafe { &(*adapter_address_it).PhysicalAddress[..len] }) + )), + }; + + // Find interface addresses + let mut current_unicast_address = unsafe { (*adapter_address_it).FirstUnicastAddress }; + + while !current_unicast_address.is_null() { + let address = unsafe { (*current_unicast_address).Address }; + + match unsafe { (*address.lpSockaddr).sa_family } { + AF_INET => { + let sockaddr: *mut SOCKADDR_IN = address.lpSockaddr as *mut SOCKADDR_IN; + let addr = make_ipv4_addr(&sockaddr); + let netmask = make_ipv4_netmask(¤t_unicast_address); + let network_interface = NetworkInterface::new_afinet( + &address_name, + addr, + netmask, + lookup_ipv4_broadcast_addr(&adapter_address_it, &sockaddr), + index, + ) + .with_mac_addr(mac_addr.clone()); + + network_interfaces.push(network_interface); } + AF_INET6 => { + let sockaddr: *mut SOCKADDR_IN6 = address.lpSockaddr as *mut SOCKADDR_IN6; + let addr = make_ipv6_addr(&sockaddr)?; + let netmask = make_ipv6_netmask(&sockaddr); + let network_interface = NetworkInterface::new_afinet6( + &address_name, + addr, + netmask, + None, + index, + ) + .with_mac_addr(mac_addr.clone()); + + network_interfaces.push(network_interface); + } + _ => {} } - if !adapter_address.is_null() { - adapter_address = unsafe { (*adapter_address).Next }; + if !current_unicast_address.is_null() { + current_unicast_address = unsafe { (*current_unicast_address).Next }; } } + + adapter_address_it = unsafe { (*adapter_address_it).Next }; } Ok(network_interfaces)