Merge "Add safety comments." into main am: 367ade7e3e am: 725d525315 (d8d954a0) · Commits · e / os / android_packages_modules_DnsResolver

doh/boot_time.rs

+6 −10

Original line number	Original line	Diff line number	Diff line
	@@ -57,8 +57,7 @@ impl BootTime {
	/// Gets a `BootTime` representing the current moment in time.		/// Gets a `BootTime` representing the current moment in time.
	pub fn now() -> BootTime {		pub fn now() -> BootTime {
	let mut t = libc::timespec { tv_sec: 0, tv_nsec: 0 };		let mut t = libc::timespec { tv_sec: 0, tv_nsec: 0 };
	// # Safety		// SAFETY: clock_gettime's only action will be to possibly write to the pointer provided,
	// clock_gettime's only action will be to possibly write to the pointer provided,
	// and no borrows exist from that object other than the &mut used to construct the pointer		// and no borrows exist from that object other than the &mut used to construct the pointer
	// itself.		// itself.
	if unsafe { libc::clock_gettime(libc::CLOCK_BOOTTIME, &mut t as *mut libc::timespec) } != 0		if unsafe { libc::clock_gettime(libc::CLOCK_BOOTTIME, &mut t as *mut libc::timespec) } != 0
	@@ -93,9 +92,8 @@ struct TimerFd(RawFd);

	impl Drop for TimerFd {		impl Drop for TimerFd {
	fn drop(&mut self) {		fn drop(&mut self) {
	// # Safety		// SAFETY: The fd is owned by the TimerFd struct, and no memory access occurs as a result of
	// The fd is owned by the TimerFd struct, and no memory access occurs as a result of this		// this call.
	// call.
	unsafe {		unsafe {
	libc::close(self.0);		libc::close(self.0);
	}		}
	@@ -110,9 +108,8 @@ impl AsRawFd for TimerFd {

	impl TimerFd {		impl TimerFd {
	fn create() -> io::Result<Self> {		fn create() -> io::Result<Self> {
	// # Unsafe		// SAFETY: This libc call will either give us back a file descriptor or fail, it does not
	// This libc call will either give us back a file descriptor or fail, it does not act on		// act on memory or resources.
	// memory or resources.
	let raw = unsafe {		let raw = unsafe {
	libc::timerfd_create(libc::CLOCK_BOOTTIME, libc::TFD_NONBLOCK \| libc::TFD_CLOEXEC)		libc::timerfd_create(libc::CLOCK_BOOTTIME, libc::TFD_NONBLOCK \| libc::TFD_CLOEXEC)
	};		};
	@@ -131,8 +128,7 @@ impl TimerFd {
	tv_nsec: duration.subsec_nanos().try_into().unwrap(),		tv_nsec: duration.subsec_nanos().try_into().unwrap(),
	},		},
	};		};
	// # Unsafe		// SAFETY: We own `timer` and there are no borrows to it other than the pointer we pass to
	// We own `timer` and there are no borrows to it other than the pointer we pass to
	// timerfd_settime. timerfd_settime is explicitly documented to handle a null output		// timerfd_settime. timerfd_settime is explicitly documented to handle a null output
	// parameter for its fourth argument by not filling out the output. The fd passed in at		// parameter for its fourth argument by not filling out the output. The fd passed in at
	// self.0 is owned by the `TimerFd` struct, so we aren't breaking anyone else's invariants.		// self.0 is owned by the `TimerFd` struct, so we aren't breaking anyone else's invariants.

doh/connection/mod.rs

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -61,7 +61,7 @@ fn new_scid() -> [u8; quiche::MAX_CONN_ID_LEN] {
	fn mark_socket(socket: &std::net::UdpSocket, socket_mark: u32) -> io::Result<()> {		fn mark_socket(socket: &std::net::UdpSocket, socket_mark: u32) -> io::Result<()> {
	use std::os::unix::io::AsRawFd;		use std::os::unix::io::AsRawFd;
	let fd = socket.as_raw_fd();		let fd = socket.as_raw_fd();
	// libc::setsockopt is a wrapper function calling into bionic setsockopt.		// SAFETY: libc::setsockopt is a wrapper function calling into bionic setsockopt.
	// The only pointer being passed in is &socket_mark, which is valid by virtue of being a		// The only pointer being passed in is &socket_mark, which is valid by virtue of being a
	// reference, and the foreign function doesn't take ownership or a reference to that memory		// reference, and the foreign function doesn't take ownership or a reference to that memory
	// after completion.		// after completion.

doh/ffi.rs

+38 −16

Original line number	Original line	Diff line number	Diff line
	@@ -35,8 +35,12 @@ use tokio::sync::oneshot;
	use tokio::task;		use tokio::task;
	use url::Url;		use url::Url;

	pub type ValidationCallback =		pub type ValidationCallback = unsafe extern "C" fn(
	extern "C" fn(net_id: uint32_t, success: bool, ip_addr: const c_char, host: const c_char);		net_id: uint32_t,
			success: bool,
			ip_addr: *const c_char,
			host: *const c_char,
			);
	pub type TagSocketCallback = extern "C" fn(sock: RawFd);		pub type TagSocketCallback = extern "C" fn(sock: RawFd);

	#[repr(C)]		#[repr(C)]
	@@ -61,7 +65,9 @@ fn wrap_validation_callback(validation_fn: ValidationCallback) -> ValidationRepo
	}		}
	};		};
	let netd_id = info.net_id;		let netd_id = info.net_id;
	task::spawn_blocking(move \|\| {		// SAFETY: The string pointers are obtained from `CString`, so they must be valid C
			// strings.
			task::spawn_blocking(move \|\| unsafe {
	validation_fn(netd_id, success, ip_addr.as_ptr(), domain.as_ptr())		validation_fn(netd_id, success, ip_addr.as_ptr(), domain.as_ptr())
	})		})
	.await		.await
	@@ -167,12 +173,16 @@ pub extern "C" fn doh_dispatcher_new(
	}		}

	/// Deletes a DoH engine created by doh_dispatcher_new().		/// Deletes a DoH engine created by doh_dispatcher_new().
			///
	/// # Safety		/// # Safety
			///
	/// `doh` must be a non-null pointer previously created by `doh_dispatcher_new()`		/// `doh` must be a non-null pointer previously created by `doh_dispatcher_new()`
	/// and not yet deleted by `doh_dispatcher_delete()`.		/// and not yet deleted by `doh_dispatcher_delete()`.
	#[no_mangle]		#[no_mangle]
	pub unsafe extern "C" fn doh_dispatcher_delete(doh: *mut DohDispatcher) {		pub unsafe extern "C" fn doh_dispatcher_delete(doh: *mut DohDispatcher) {
	Box::from_raw(doh).lock().exit_handler()		// SAFETY: The caller guarantees that `doh` was created by `doh_dispatcher_new` (which does so
			// using `Box::into_raw`), and that it hasn't yet been deleted by this function.
			unsafe { Box::from_raw(doh) }.lock().exit_handler()
	}		}

	/// Probes and stores the DoH server with the given configurations.		/// Probes and stores the DoH server with the given configurations.
	@@ -194,12 +204,15 @@ pub unsafe extern "C" fn doh_net_new(
	network_type: uint32_t,		network_type: uint32_t,
	private_dns_mode: uint32_t,		private_dns_mode: uint32_t,
	) -> int32_t {		) -> int32_t {
	let (url, domain, ip_addr, cert_path) = match (		// SAFETY: The caller guarantees that these are all valid nul-terminated C strings.
			let (url, domain, ip_addr, cert_path) = match unsafe {
			(
	std::ffi::CStr::from_ptr(url).to_str(),		std::ffi::CStr::from_ptr(url).to_str(),
	std::ffi::CStr::from_ptr(domain).to_str(),		std::ffi::CStr::from_ptr(domain).to_str(),
	std::ffi::CStr::from_ptr(ip_addr).to_str(),		std::ffi::CStr::from_ptr(ip_addr).to_str(),
	std::ffi::CStr::from_ptr(cert_path).to_str(),		std::ffi::CStr::from_ptr(cert_path).to_str(),
	) {		)
			} {
	(Ok(url), Ok(domain), Ok(ip_addr), Ok(cert_path)) => {		(Ok(url), Ok(domain), Ok(ip_addr), Ok(cert_path)) => {
	if domain.is_empty() {		if domain.is_empty() {
	(url, None, ip_addr.to_string(), None)		(url, None, ip_addr.to_string(), None)
	@@ -268,7 +281,9 @@ pub unsafe extern "C" fn doh_query(
	response_len: size_t,		response_len: size_t,
	timeout_ms: uint64_t,		timeout_ms: uint64_t,
	) -> ssize_t {		) -> ssize_t {
	let q = slice::from_raw_parts_mut(dns_query, dns_query_len);		// SAFETY: The caller guarantees that `dns_query` is a valid pointer to a buffer of at least
			// `dns_query_len` items.
			let q = unsafe { slice::from_raw_parts_mut(dns_query, dns_query_len) };

	let (resp_tx, resp_rx) = oneshot::channel();		let (resp_tx, resp_rx) = oneshot::channel();
	let t = Duration::from_millis(timeout_ms);		let t = Duration::from_millis(timeout_ms);
	@@ -298,7 +313,10 @@ pub unsafe extern "C" fn doh_query(
	if answer.len() > response_len \|\| answer.len() > isize::MAX as usize {		if answer.len() > response_len \|\| answer.len() > isize::MAX as usize {
	return DOH_RESULT_INTERNAL_ERROR;		return DOH_RESULT_INTERNAL_ERROR;
	}		}
	let response = slice::from_raw_parts_mut(response, answer.len());		// SAFETY: The caller guarantees that response points to a valid buffer at
			// least `response_len` long, and we just checked that `answer.len()` is no
			// longer than `response_len`.
			let response = unsafe { slice::from_raw_parts_mut(response, answer.len()) };
	response.copy_from_slice(&answer);		response.copy_from_slice(&answer);
	answer.len() as ssize_t		answer.len() as ssize_t
	}		}
	@@ -341,25 +359,27 @@ mod tests {
	const LOOPBACK_ADDR: &str = "127.0.0.1:443";		const LOOPBACK_ADDR: &str = "127.0.0.1:443";
	const LOCALHOST_URL: &str = "https://mylocal.com/dns-query";		const LOCALHOST_URL: &str = "https://mylocal.com/dns-query";

	extern "C" fn success_cb(		unsafe extern "C" fn success_cb(
	net_id: uint32_t,		net_id: uint32_t,
	success: bool,		success: bool,
	ip_addr: *const c_char,		ip_addr: *const c_char,
	host: *const c_char,		host: *const c_char,
	) {		) {
	assert!(success);		assert!(success);
			// SAFETY: The caller guarantees that ip_addr and host are valid nul-terminated C strings.
	unsafe {		unsafe {
	assert_validation_info(net_id, ip_addr, host);		assert_validation_info(net_id, ip_addr, host);
	}		}
	}		}

	extern "C" fn fail_cb(		unsafe extern "C" fn fail_cb(
	net_id: uint32_t,		net_id: uint32_t,
	success: bool,		success: bool,
	ip_addr: *const c_char,		ip_addr: *const c_char,
	host: *const c_char,		host: *const c_char,
	) {		) {
	assert!(!success);		assert!(!success);
			// SAFETY: The caller guarantees that ip_addr and host are valid nul-terminated C strings.
	unsafe {		unsafe {
	assert_validation_info(net_id, ip_addr, host);		assert_validation_info(net_id, ip_addr, host);
	}		}
	@@ -373,10 +393,12 @@ mod tests {
	host: *const c_char,		host: *const c_char,
	) {		) {
	assert_eq!(net_id, TEST_NET_ID);		assert_eq!(net_id, TEST_NET_ID);
	let ip_addr = std::ffi::CStr::from_ptr(ip_addr).to_str().unwrap();		// SAFETY: The caller guarantees that `ip_addr` is a valid nul-terminated C string.
			let ip_addr = unsafe { std::ffi::CStr::from_ptr(ip_addr) }.to_str().unwrap();
	let expected_addr: SocketAddr = LOOPBACK_ADDR.parse().unwrap();		let expected_addr: SocketAddr = LOOPBACK_ADDR.parse().unwrap();
	assert_eq!(ip_addr, expected_addr.ip().to_string());		assert_eq!(ip_addr, expected_addr.ip().to_string());
	let host = std::ffi::CStr::from_ptr(host).to_str().unwrap();		// SAFETY: The caller guarantees that `host` is a valid nul-terminated C string.
			let host = unsafe { std::ffi::CStr::from_ptr(host) }.to_str().unwrap();
	assert_eq!(host, "");		assert_eq!(host, "");
	}		}

doh/tests/doh_frontend/src/dns_https_frontend.rs

+3 −0

Original line number	Original line	Diff line number	Diff line
	@@ -477,6 +477,9 @@ fn into_tokio_udp_socket(socket: std::net::UdpSocket) -> Result<UdpSocket> {

	fn build_pipe() -> Result<(File, File)> {		fn build_pipe() -> Result<(File, File)> {
	let mut fds = [0, 0];		let mut fds = [0, 0];
			// SAFETY: The pointer we pass to `pipe` must be valid because it comes from a reference. The
			// file descriptors it returns must be valid and open, so they are safe to pass to
			// `File::from_raw_fd`.
	unsafe {		unsafe {
	if libc::pipe(fds.as_mut_ptr()) == 0 {		if libc::pipe(fds.as_mut_ptr()) == 0 {
	return Ok((File::from_raw_fd(fds[0]), File::from_raw_fd(fds[1])));		return Ok((File::from_raw_fd(fds[0]), File::from_raw_fd(fds[1])));

doh/tests/doh_frontend/src/ffi.rs

+20 −8

Original line number	Original line	Diff line number	Diff line
	@@ -38,10 +38,14 @@ pub unsafe extern "C" fn frontend_new(
	backend_addr: *const c_char,		backend_addr: *const c_char,
	backend_port: *const c_char,		backend_port: *const c_char,
	) -> *mut DohFrontend {		) -> *mut DohFrontend {
	let addr = CStr::from_ptr(addr).to_str().unwrap();		// SAFETY: Our caller promises that addr is a valid C string.
	let port = CStr::from_ptr(port).to_str().unwrap();		let addr = unsafe { CStr::from_ptr(addr) }.to_str().unwrap();
	let backend_addr = CStr::from_ptr(backend_addr).to_str().unwrap();		// SAFETY: Our caller promises that port is a valid C string.
	let backend_port = CStr::from_ptr(backend_port).to_str().unwrap();		let port = unsafe { CStr::from_ptr(port) }.to_str().unwrap();
			// SAFETY: Our caller promises that backend_addr is a valid C string.
			let backend_addr = unsafe { CStr::from_ptr(backend_addr) }.to_str().unwrap();
			// SAFETY: Our caller promises that backend_port is a valid C string.
			let backend_port = unsafe { CStr::from_ptr(backend_port) }.to_str().unwrap();

	let socket_addr = to_socket_addr(addr, port).or_else(logging_and_return_err);		let socket_addr = to_socket_addr(addr, port).or_else(logging_and_return_err);
	let backend_socket_addr =		let backend_socket_addr =
	@@ -73,13 +77,19 @@ pub extern "C" fn frontend_stop(doh: &mut DohFrontend) -> bool {
	/// If the caller has called `frontend_start` to start `DohFrontend`, it has to call		/// If the caller has called `frontend_start` to start `DohFrontend`, it has to call
	/// call `frontend_stop` to stop the worker thread before deleting the object.		/// call `frontend_stop` to stop the worker thread before deleting the object.
	///		///
			/// The DohFrontend is not set to null pointer, caller needs to do it on its own.
			///
	/// # Safety		/// # Safety
	///		///
	/// The DohFrontend is not set to null pointer, caller needs to do it on its own.		/// `doh` must be a pointer either null or previously returned by `frontend_new`, and not yet passed
			/// to `frontend_delete`.
	#[no_mangle]		#[no_mangle]
	pub unsafe extern "C" fn frontend_delete(doh: *mut DohFrontend) {		pub unsafe extern "C" fn frontend_delete(doh: *mut DohFrontend) {
	if !doh.is_null() {		if !doh.is_null() {
	drop(Box::from_raw(doh));		// SAFETY: Our caller promised that `doh` was either null or previously returned by
			// `frontend_new`. We just checked that it's not null, so it must have been returned by
			// `frontend_new`, which obtained it from `Box::into_raw`.
			drop(unsafe { Box::from_raw(doh) });
	}		}
	}		}

	@@ -96,7 +106,8 @@ pub unsafe extern "C" fn frontend_set_certificate(
	if certificate.is_null() {		if certificate.is_null() {
	return false;		return false;
	}		}
	let certificate = CStr::from_ptr(certificate).to_str().unwrap();		// SAFETY: Our caller promises that certificate is a valid C string.
			let certificate = unsafe { CStr::from_ptr(certificate) }.to_str().unwrap();
	doh.set_certificate(certificate).or_else(logging_and_return_err).is_ok()		doh.set_certificate(certificate).or_else(logging_and_return_err).is_ok()
	}		}

	@@ -113,7 +124,8 @@ pub unsafe extern "C" fn frontend_set_private_key(
	if private_key.is_null() {		if private_key.is_null() {
	return false;		return false;
	}		}
	let private_key = CStr::from_ptr(private_key).to_str().unwrap();		// SAFETY: Our caller promises that private_key is a valid C string.
			let private_key = unsafe { CStr::from_ptr(private_key) }.to_str().unwrap();
	doh.set_private_key(private_key).or_else(logging_and_return_err).is_ok()		doh.set_private_key(private_key).or_else(logging_and_return_err).is_ok()
	}		}