diff options
author | Chirantan Ekbote <chirantan@chromium.org> | 2018-06-18 15:52:48 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2018-06-21 19:56:34 -0700 |
commit | f2fb37e669f75a8560310c501f04f1a06c74b5ff (patch) | |
tree | f6c8fae612527e4cd20e44bc483d8e60b0f27919 | |
parent | 298b41cf82dc807b21e0352c6658c52d5550374d (diff) | |
download | crosvm-f2fb37e669f75a8560310c501f04f1a06c74b5ff.tar crosvm-f2fb37e669f75a8560310c501f04f1a06c74b5ff.tar.gz crosvm-f2fb37e669f75a8560310c501f04f1a06c74b5ff.tar.bz2 crosvm-f2fb37e669f75a8560310c501f04f1a06c74b5ff.tar.lz crosvm-f2fb37e669f75a8560310c501f04f1a06c74b5ff.tar.xz crosvm-f2fb37e669f75a8560310c501f04f1a06c74b5ff.tar.zst crosvm-f2fb37e669f75a8560310c501f04f1a06c74b5ff.zip |
p9: Fix unsafe block in read function
We calculate the size of the buffer to allocate for a read message by finding the minimum of the requested size and the number of bytes left in the response message buffer. However, we then turned around and used an unsafe block to set the length of the allocated buffer to the requested size rather than the calculated size. This could lead to memory corruption if the file we were trying to read had enough bytes to fill up the whole buffer and the requested size was larger than the max message size. Replace both unsafe blocks with a resize function instead. The compiler is smart enough to turn this into a memset and zero-initializing a few KB of memory is not that expensive. BUG=chromium:703939 TEST=none Change-Id: Ia9911d4176322bc9af0753541bd29d7a4723503b Signed-off-by: Chirantan Ekbote <chirantan@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1110479 Reviewed-by: Dylan Reid <dgreid@chromium.org>
-rw-r--r-- | p9/src/server.rs | 20 |
1 files changed, 6 insertions, 14 deletions
diff --git a/p9/src/server.rs b/p9/src/server.rs index ee7266a..9c41941 100644 --- a/p9/src/server.rs +++ b/p9/src/server.rs @@ -401,22 +401,14 @@ impl Server { data: Data(Vec::new()), }), }.byte_size(); - let count = min(self.msize - header_size, read.count); - let mut buf = Data(Vec::with_capacity(count as usize)); - - // Safe because `buf` is guaranteed to have a capacity of `read.count`. We do - // this because we don't want to spend time zero-initializing a potentially - // large buffer. - unsafe { - buf.set_len(read.count as usize); - } + + let capacity = min(self.msize - header_size, read.count); + let mut buf = Data(Vec::with_capacity(capacity as usize)); + buf.resize(capacity as usize, 0); + let count = file.read_at(&mut buf, read.offset)?; + buf.resize(count, 0); - // Safe because read_at guarantees that `count` bytes have been written - // into `buf` and that `count` is less than or equal to `buf.len()`. - unsafe { - buf.set_len(count); - } Ok(Rmessage::Read(Rread { data: buf })) } |