Skip to content

Commit 7c2466e

Browse files
committed
Linux: Cleanup: Trim unsafe blocks in recv()
This requires some shuffling around of declarations, to faciliate untangling the actually unsafe operations from those not affecting safety. (As far as reasonably possible...) Also added a new assertion to make sure that the trimmed `unsafe` blocks really do not rely on any conditions being upheld outside.
1 parent 0bda93a commit 7c2466e

File tree

1 file changed

+43
-34
lines changed

1 file changed

+43
-34
lines changed

platform/linux/mod.rs

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -733,14 +733,18 @@ enum BlockingMode {
733733

734734
fn recv(fd: c_int, blocking_mode: BlockingMode)
735735
-> Result<(Vec<u8>, Vec<OpaqueUnixChannel>, Vec<UnixSharedMemory>),UnixError> {
736+
737+
let (mut channels, mut shared_memory_regions) = (Vec::new(), Vec::new());
738+
739+
// First fragments begins with a header recording the total data length.
740+
//
741+
// We use this to determine whether we already got the entire message,
742+
// or need to receive additional fragments -- and if so, how much.
743+
let mut total_size = 0usize;
744+
let mut main_data_buffer;
736745
unsafe {
737-
// First fragment begins with a header recording the total data length.
738-
//
739-
// We use this to determine whether we already got the entire message,
740-
// or need to receive additional fragments -- and if so, how much.
741-
let mut total_size = 0usize;
742746
// Allocate a buffer without initialising the memory.
743-
let mut main_data_buffer = Vec::with_capacity(UnixSender::get_max_fragment_size());
747+
main_data_buffer = Vec::with_capacity(UnixSender::get_max_fragment_size());
744748
main_data_buffer.set_len(UnixSender::get_max_fragment_size());
745749

746750
let iovec = [
@@ -765,7 +769,6 @@ fn recv(fd: c_int, blocking_mode: BlockingMode)
765769
} else {
766770
(cmsg.cmsg_len() - mem::size_of::<cmsghdr>()) / mem::size_of::<c_int>()
767771
};
768-
let (mut channels, mut shared_memory_regions) = (Vec::new(), Vec::new());
769772
for index in 0..channel_length {
770773
let fd = *cmsg_fds.offset(index as isize);
771774
if is_socket(fd) {
@@ -774,30 +777,35 @@ fn recv(fd: c_int, blocking_mode: BlockingMode)
774777
}
775778
shared_memory_regions.push(UnixSharedMemory::from_fd(fd));
776779
}
780+
}
777781

778-
if total_size == main_data_buffer.len() {
779-
// Fast path: no fragments.
780-
return Ok((main_data_buffer, channels, shared_memory_regions))
781-
}
782+
if total_size == main_data_buffer.len() {
783+
// Fast path: no fragments.
784+
return Ok((main_data_buffer, channels, shared_memory_regions))
785+
}
782786

783-
// Reassemble fragments.
784-
//
785-
// The initial fragment carries the receive end of a dedicated channel
786-
// through which all the remaining fragments will be coming in.
787-
let dedicated_rx = channels.pop().unwrap().to_receiver();
788-
789-
// Extend the buffer to hold the entire message, without initialising the memory.
790-
let len = main_data_buffer.len();
791-
main_data_buffer.reserve(total_size - len);
792-
793-
// Receive followup fragments directly into the main buffer.
794-
while main_data_buffer.len() < total_size {
795-
let write_pos = main_data_buffer.len();
796-
let end_pos = cmp::min(write_pos + UnixSender::fragment_size(*SYSTEM_SENDBUF_SIZE),
797-
total_size);
787+
// Reassemble fragments.
788+
//
789+
// The initial fragment carries the receive end of a dedicated channel
790+
// through which all the remaining fragments will be coming in.
791+
let dedicated_rx = channels.pop().unwrap().to_receiver();
792+
793+
// Extend the buffer to hold the entire message, without initialising the memory.
794+
let len = main_data_buffer.len();
795+
main_data_buffer.reserve(total_size - len);
796+
797+
// Receive followup fragments directly into the main buffer.
798+
while main_data_buffer.len() < total_size {
799+
let write_pos = main_data_buffer.len();
800+
let end_pos = cmp::min(write_pos + UnixSender::fragment_size(*SYSTEM_SENDBUF_SIZE),
801+
total_size);
802+
let result = unsafe {
798803
assert!(end_pos <= main_data_buffer.capacity());
799804
main_data_buffer.set_len(end_pos);
800805

806+
// Integer underflow could make the following code unsound...
807+
assert!(end_pos >= write_pos);
808+
801809
// Note: we always use blocking mode for followup fragments,
802810
// to make sure that once we start receiving a multi-fragment message,
803811
// we don't abort in the middle of it...
@@ -806,16 +814,17 @@ fn recv(fd: c_int, blocking_mode: BlockingMode)
806814
end_pos - write_pos,
807815
0);
808816
main_data_buffer.set_len(write_pos + cmp::max(result, 0) as usize);
817+
result
818+
};
809819

810-
if result == 0 {
811-
return Err(UnixError(libc::ECONNRESET))
812-
} else if result < 0 {
813-
return Err(UnixError::last())
814-
};
815-
}
816-
817-
Ok((main_data_buffer, channels, shared_memory_regions))
820+
if result == 0 {
821+
return Err(UnixError(libc::ECONNRESET))
822+
} else if result < 0 {
823+
return Err(UnixError::last())
824+
};
818825
}
826+
827+
Ok((main_data_buffer, channels, shared_memory_regions))
819828
}
820829

821830
#[cfg(target_os="android")]

0 commit comments

Comments
 (0)