Loading changes/bug25127 0 → 100644 +7 −0 Original line number Diff line number Diff line o Minor bugfixes (Rust FFI): - Fix a minor memory leak which would happen whenever the C code would call the Rust implementation of protover_get_supported_protocols(). This was due to the C version returning a static string, whereas the Rust version newly allocated a CString to pass accross the FFI boundary. Consequently, the C code was not expecting to need to free() what it was given. Fixes bug 25127; bugfix on 0.3.2.1-alpha. src/rust/protover/ffi.rs +18 −11 Original line number Diff line number Diff line Loading @@ -137,18 +137,25 @@ pub extern "C" fn protocol_list_supports_protocol_or_later( /// Provide an interface for C to translate arguments and return types for /// protover::get_supported_protocols #[no_mangle] pub extern "C" fn protover_get_supported_protocols() -> *mut c_char { // Not handling errors when unwrapping as the content is controlled // and is an empty string let empty = CString::new("").unwrap(); let supported = get_supported_protocols(); let c_supported = match CString::new(supported) { Ok(n) => n, Err(_) => return empty.into_raw(), }; pub extern "C" fn protover_get_supported_protocols() -> *const c_char { let supported: &'static CStr; // If we're going to pass it to C, there cannot be any intermediate NUL // bytes. An assert is okay here, since changing the const byte slice // in protover.rs to contain a NUL byte somewhere in the middle would be a // programming error. assert!(!SUPPORTED_PROTOCOLS[..SUPPORTED_PROTOCOLS.len() - 1].contains(&0x00)); assert!(SUPPORTED_PROTOCOLS[SUPPORTED_PROTOCOLS.len() - 1] == 0x00); // It's okay to call the "unchecked" version of the function because // we can see that the bytes we're passing into it 1) are valid UTF-8, // 2) have no intermediate NUL bytes, and 3) are terminated with a NUL // byte. unsafe { supported = CStr::from_bytes_with_nul_unchecked(SUPPORTED_PROTOCOLS); } c_supported.into_raw() supported.as_ptr() } /// Provide an interface for C to translate arguments and return types for Loading src/rust/protover/protover.rs +30 −17 Original line number Diff line number Diff line Loading @@ -3,12 +3,14 @@ use external::c_tor_version_as_new_as; use std::str; use std::str::FromStr; use std::fmt; use std::collections::{HashMap, HashSet}; use std::ops::Range; use std::string::String; /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. /// Loading @@ -22,21 +24,29 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` const MAX_PROTOCOLS_TO_EXPAND: u32 = 500; /// Currently supported protocols and their versions /// Currently supported protocols and their versions, as a byte-slice. /// /// # Warning /// /// This byte-slice ends in a NUL byte. This is so that we can directly convert /// it to an `&'static CStr` in the FFI code, in order to hand the static string /// to C in a way that is compatible with C static strings. /// /// Rust code which wishes to accesses this string should use /// `protover::get_supported_protocols()` instead. /// /// C_RUST_COUPLED: src/or/protover.c `protover_get_supported_protocols` const SUPPORTED_PROTOCOLS: &'static [&'static str] = &[ "Cons=1-2", "Desc=1-2", "DirCache=1-2", "HSDir=1-2", "HSIntro=3-4", "HSRend=1-2", "Link=1-5", "LinkAuth=1,3", "Microdesc=1-2", "Relay=1-2", ]; pub(crate) const SUPPORTED_PROTOCOLS: &'static [u8] = b"Cons=1-2 \ Desc=1-2 \ DirCache=1-2 \ HSDir=1-2 \ HSIntro=3-4 \ HSRend=1-2 \ Link=1-5 \ LinkAuth=1,3 \ Microdesc=1-2 \ Relay=1-2\0"; /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// Loading Loading @@ -94,8 +104,11 @@ impl FromStr for Proto { /// /// "HSDir=1-1 LinkAuth=1" /// pub fn get_supported_protocols() -> String { SUPPORTED_PROTOCOLS.join(" ") pub fn get_supported_protocols() -> &'static str { unsafe { // The `len() - 1` is to remove the NUL byte. str::from_utf8_unchecked(&SUPPORTED_PROTOCOLS[..SUPPORTED_PROTOCOLS.len() - 1]) } } /// Translates a vector representation of a protocol list into a HashMap Loading Loading @@ -134,7 +147,7 @@ fn parse_protocols_from_string<'a>( /// of the error. /// fn tor_supported() -> Result<HashMap<Proto, HashSet<u32>>, &'static str> { parse_protocols(SUPPORTED_PROTOCOLS.iter()) parse_protocols(get_supported_protocols().split(" ")) } /// Get the unique version numbers supported by a subprotocol. Loading Loading @@ -625,7 +638,7 @@ pub fn compute_vote( } let mut final_output: HashMap<String, String> = HashMap::with_capacity(SUPPORTED_PROTOCOLS.len()); HashMap::with_capacity(get_supported_protocols().split(" ").count()); // Go through and remove verstions that are less than the threshold for (protocol, versions) in all_count { Loading Loading
changes/bug25127 0 → 100644 +7 −0 Original line number Diff line number Diff line o Minor bugfixes (Rust FFI): - Fix a minor memory leak which would happen whenever the C code would call the Rust implementation of protover_get_supported_protocols(). This was due to the C version returning a static string, whereas the Rust version newly allocated a CString to pass accross the FFI boundary. Consequently, the C code was not expecting to need to free() what it was given. Fixes bug 25127; bugfix on 0.3.2.1-alpha.
src/rust/protover/ffi.rs +18 −11 Original line number Diff line number Diff line Loading @@ -137,18 +137,25 @@ pub extern "C" fn protocol_list_supports_protocol_or_later( /// Provide an interface for C to translate arguments and return types for /// protover::get_supported_protocols #[no_mangle] pub extern "C" fn protover_get_supported_protocols() -> *mut c_char { // Not handling errors when unwrapping as the content is controlled // and is an empty string let empty = CString::new("").unwrap(); let supported = get_supported_protocols(); let c_supported = match CString::new(supported) { Ok(n) => n, Err(_) => return empty.into_raw(), }; pub extern "C" fn protover_get_supported_protocols() -> *const c_char { let supported: &'static CStr; // If we're going to pass it to C, there cannot be any intermediate NUL // bytes. An assert is okay here, since changing the const byte slice // in protover.rs to contain a NUL byte somewhere in the middle would be a // programming error. assert!(!SUPPORTED_PROTOCOLS[..SUPPORTED_PROTOCOLS.len() - 1].contains(&0x00)); assert!(SUPPORTED_PROTOCOLS[SUPPORTED_PROTOCOLS.len() - 1] == 0x00); // It's okay to call the "unchecked" version of the function because // we can see that the bytes we're passing into it 1) are valid UTF-8, // 2) have no intermediate NUL bytes, and 3) are terminated with a NUL // byte. unsafe { supported = CStr::from_bytes_with_nul_unchecked(SUPPORTED_PROTOCOLS); } c_supported.into_raw() supported.as_ptr() } /// Provide an interface for C to translate arguments and return types for Loading
src/rust/protover/protover.rs +30 −17 Original line number Diff line number Diff line Loading @@ -3,12 +3,14 @@ use external::c_tor_version_as_new_as; use std::str; use std::str::FromStr; use std::fmt; use std::collections::{HashMap, HashSet}; use std::ops::Range; use std::string::String; /// The first version of Tor that included "proto" entries in its descriptors. /// Authorities should use this to decide whether to guess proto lines. /// Loading @@ -22,21 +24,29 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` const MAX_PROTOCOLS_TO_EXPAND: u32 = 500; /// Currently supported protocols and their versions /// Currently supported protocols and their versions, as a byte-slice. /// /// # Warning /// /// This byte-slice ends in a NUL byte. This is so that we can directly convert /// it to an `&'static CStr` in the FFI code, in order to hand the static string /// to C in a way that is compatible with C static strings. /// /// Rust code which wishes to accesses this string should use /// `protover::get_supported_protocols()` instead. /// /// C_RUST_COUPLED: src/or/protover.c `protover_get_supported_protocols` const SUPPORTED_PROTOCOLS: &'static [&'static str] = &[ "Cons=1-2", "Desc=1-2", "DirCache=1-2", "HSDir=1-2", "HSIntro=3-4", "HSRend=1-2", "Link=1-5", "LinkAuth=1,3", "Microdesc=1-2", "Relay=1-2", ]; pub(crate) const SUPPORTED_PROTOCOLS: &'static [u8] = b"Cons=1-2 \ Desc=1-2 \ DirCache=1-2 \ HSDir=1-2 \ HSIntro=3-4 \ HSRend=1-2 \ Link=1-5 \ LinkAuth=1,3 \ Microdesc=1-2 \ Relay=1-2\0"; /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// Loading Loading @@ -94,8 +104,11 @@ impl FromStr for Proto { /// /// "HSDir=1-1 LinkAuth=1" /// pub fn get_supported_protocols() -> String { SUPPORTED_PROTOCOLS.join(" ") pub fn get_supported_protocols() -> &'static str { unsafe { // The `len() - 1` is to remove the NUL byte. str::from_utf8_unchecked(&SUPPORTED_PROTOCOLS[..SUPPORTED_PROTOCOLS.len() - 1]) } } /// Translates a vector representation of a protocol list into a HashMap Loading Loading @@ -134,7 +147,7 @@ fn parse_protocols_from_string<'a>( /// of the error. /// fn tor_supported() -> Result<HashMap<Proto, HashSet<u32>>, &'static str> { parse_protocols(SUPPORTED_PROTOCOLS.iter()) parse_protocols(get_supported_protocols().split(" ")) } /// Get the unique version numbers supported by a subprotocol. Loading Loading @@ -625,7 +638,7 @@ pub fn compute_vote( } let mut final_output: HashMap<String, String> = HashMap::with_capacity(SUPPORTED_PROTOCOLS.len()); HashMap::with_capacity(get_supported_protocols().split(" ").count()); // Go through and remove verstions that are less than the threshold for (protocol, versions) in all_count { Loading