From 85e936d4bde6959eb0276e685091bfdd9776467c Mon Sep 17 00:00:00 2001
From: Sergio Lopez <slp@redhat.com>
Date: Wed, 23 Oct 2019 16:07:03 +0200
Subject: [PATCH] vhost_rs: fix VhostUserConfig payload management

The VhostUserConfig carries a message with a payload, the contents of
which depend on the kind of device being emulated.

With this change, we calculate the offset of the payload within the
message, check its size corresponds to the expected one, and pass it
to the backend as a reference to a slice adjusted to the payload
dimensions.

The backend will be responsible of validating the payload, as it's the
one aware of its expected contents.

Signed-off-by: Sergio Lopez <slp@redhat.com>
---
 vhost_rs/src/vhost_user/slave_req_handler.rs | 14 ++++++++-----
 vhost_user_backend/src/lib.rs                | 22 --------------------
 2 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/vhost_rs/src/vhost_user/slave_req_handler.rs b/vhost_rs/src/vhost_user/slave_req_handler.rs
index 934c6d498..480689f7f 100644
--- a/vhost_rs/src/vhost_user/slave_req_handler.rs
+++ b/vhost_rs/src/vhost_user/slave_req_handler.rs
@@ -265,7 +265,6 @@ impl<S: VhostUserSlaveReqHandler> SlaveReqHandler<S> {
                 if self.acked_protocol_features & VhostUserProtocolFeatures::CONFIG.bits() == 0 {
                     return Err(Error::InvalidOperation);
                 }
-                self.check_request_size(&hdr, size, mem::size_of::<VhostUserConfig>())?;
                 self.get_config(&hdr, &buf)?;
             }
             MasterReq::SET_CONFIG => {
@@ -341,6 +340,10 @@ impl<S: VhostUserSlaveReqHandler> SlaveReqHandler<S> {
         if !msg.is_valid() {
             return Err(Error::InvalidMessage);
         }
+        let payload_offset = mem::size_of::<VhostUserConfig>();
+        if buf.len() - payload_offset != msg.size as usize {
+            return Err(Error::InvalidMessage);
+        }
         let flags = match VhostUserConfigFlags::from_bits(msg.flags) {
             Some(val) => val,
             None => return Err(Error::InvalidMessage),
@@ -519,6 +522,7 @@ impl<S: VhostUserSlaveReqHandler> SlaveReqHandler<S> {
     fn new_reply_header<T: Sized>(
         &self,
         req: &VhostUserMsgHeader<MasterReq>,
+        payload_size: usize,
     ) -> Result<VhostUserMsgHeader<MasterReq>> {
         if mem::size_of::<T>() > MAX_MSG_SIZE {
             return Err(Error::InvalidParam);
@@ -527,7 +531,7 @@ impl<S: VhostUserSlaveReqHandler> SlaveReqHandler<S> {
         Ok(VhostUserMsgHeader::new(
             req.get_code(),
             VhostUserHeaderFlag::REPLY.bits(),
-            mem::size_of::<T>() as u32,
+            (mem::size_of::<T>() + payload_size) as u32,
         ))
     }
 
@@ -537,7 +541,7 @@ impl<S: VhostUserSlaveReqHandler> SlaveReqHandler<S> {
         res: Result<()>,
     ) -> Result<()> {
         if self.reply_ack_enabled {
-            let hdr = self.new_reply_header::<VhostUserU64>(req)?;
+            let hdr = self.new_reply_header::<VhostUserU64>(req, 0)?;
             let val = match res {
                 Ok(_) => 0,
                 Err(_) => 1,
@@ -553,7 +557,7 @@ impl<S: VhostUserSlaveReqHandler> SlaveReqHandler<S> {
         req: &VhostUserMsgHeader<MasterReq>,
         msg: &T,
     ) -> Result<()> {
-        let hdr = self.new_reply_header::<T>(req)?;
+        let hdr = self.new_reply_header::<T>(req, 0)?;
         self.main_sock.send_message(&hdr, msg, None)?;
         Ok(())
     }
@@ -568,7 +572,7 @@ impl<S: VhostUserSlaveReqHandler> SlaveReqHandler<S> {
         T: Sized,
         P: Sized,
     {
-        let hdr = self.new_reply_header::<T>(req)?;
+        let hdr = self.new_reply_header::<T>(req, payload.len())?;
         self.main_sock
             .send_message_with_payload(&hdr, msg, payload, None)?;
         Ok(())
diff --git a/vhost_user_backend/src/lib.rs b/vhost_user_backend/src/lib.rs
index 3889245b2..8f34a1555 100644
--- a/vhost_user_backend/src/lib.rs
+++ b/vhost_user_backend/src/lib.rs
@@ -15,7 +15,6 @@ use std::thread;
 use vhost_rs::vhost_user::message::{
     VhostUserConfigFlags, VhostUserMemoryRegion, VhostUserProtocolFeatures,
     VhostUserVirtioFeatures, VhostUserVringAddrFlags, VhostUserVringState,
-    VHOST_USER_CONFIG_OFFSET, VHOST_USER_CONFIG_SIZE,
 };
 use vhost_rs::vhost_user::{
     Error as VhostUserError, Result as VhostUserResult, SlaveListener, VhostUserSlaveReqHandler,
@@ -720,16 +719,6 @@ impl<S: VhostUserBackend> VhostUserSlaveReqHandler for VhostUserHandler<S> {
         size: u32,
         _flags: VhostUserConfigFlags,
     ) -> VhostUserResult<Vec<u8>> {
-        if self.acked_features & VhostUserProtocolFeatures::CONFIG.bits() == 0 {
-            return Err(VhostUserError::InvalidOperation);
-        } else if offset < VHOST_USER_CONFIG_OFFSET
-            || offset >= VHOST_USER_CONFIG_SIZE
-            || size > VHOST_USER_CONFIG_SIZE - VHOST_USER_CONFIG_OFFSET
-            || size + offset > VHOST_USER_CONFIG_SIZE
-        {
-            return Err(VhostUserError::InvalidParam);
-        }
-
         Ok(self.backend.read().unwrap().get_config(offset, size))
     }
 
@@ -739,17 +728,6 @@ impl<S: VhostUserBackend> VhostUserSlaveReqHandler for VhostUserHandler<S> {
         buf: &[u8],
         _flags: VhostUserConfigFlags,
     ) -> VhostUserResult<()> {
-        let size = buf.len() as u32;
-        if self.acked_features & VhostUserProtocolFeatures::CONFIG.bits() == 0 {
-            return Err(VhostUserError::InvalidOperation);
-        } else if offset < VHOST_USER_CONFIG_OFFSET
-            || offset >= VHOST_USER_CONFIG_SIZE
-            || size > VHOST_USER_CONFIG_SIZE - VHOST_USER_CONFIG_OFFSET
-            || size + offset > VHOST_USER_CONFIG_SIZE
-        {
-            return Err(VhostUserError::InvalidParam);
-        }
-
         self.backend
             .write()
             .unwrap()