mirror of
https://github.com/cloud-hypervisor/cloud-hypervisor.git
synced 2025-02-02 01:45:21 +00:00
virtio-devices: block: Return error to driver on writes if read-only
TEST=Boot `--disk readonly=on` along with a guest that tries to write (unmodified hypervisor-fw) and observe that the virtio device thread no longer panics. Fixes: #4888 Signed-off-by: Rob Bradford <robert.bradford@intel.com>
This commit is contained in:
parent
b07d471d4f
commit
149e424b6e
@ -103,6 +103,7 @@ struct BlockEpollHandler {
|
|||||||
request_list: HashMap<u16, Request>,
|
request_list: HashMap<u16, Request>,
|
||||||
rate_limiter: Option<RateLimiter>,
|
rate_limiter: Option<RateLimiter>,
|
||||||
access_platform: Option<Arc<dyn AccessPlatform>>,
|
access_platform: Option<Arc<dyn AccessPlatform>>,
|
||||||
|
read_only: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl BlockEpollHandler {
|
impl BlockEpollHandler {
|
||||||
@ -115,6 +116,27 @@ impl BlockEpollHandler {
|
|||||||
let mut request = Request::parse(&mut desc_chain, self.access_platform.as_ref())
|
let mut request = Request::parse(&mut desc_chain, self.access_platform.as_ref())
|
||||||
.map_err(Error::RequestParsing)?;
|
.map_err(Error::RequestParsing)?;
|
||||||
|
|
||||||
|
// For virtio spec compliance
|
||||||
|
// "A device MUST set the status byte to VIRTIO_BLK_S_IOERR for a write request
|
||||||
|
// if the VIRTIO_BLK_F_RO feature if offered, and MUST NOT write any data."
|
||||||
|
if self.read_only
|
||||||
|
&& (request.request_type == RequestType::Out
|
||||||
|
|| request.request_type == RequestType::Flush)
|
||||||
|
{
|
||||||
|
desc_chain
|
||||||
|
.memory()
|
||||||
|
.write_obj(VIRTIO_BLK_S_IOERR, request.status_addr)
|
||||||
|
.map_err(Error::RequestStatus)?;
|
||||||
|
|
||||||
|
// If no asynchronous operation has been submitted, we can
|
||||||
|
// simply return the used descriptor.
|
||||||
|
queue
|
||||||
|
.add_used(desc_chain.memory(), desc_chain.head_index(), 0)
|
||||||
|
.map_err(Error::QueueAddUsed)?;
|
||||||
|
used_descs = true;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
if let Some(rate_limiter) = &mut self.rate_limiter {
|
if let Some(rate_limiter) = &mut self.rate_limiter {
|
||||||
// If limiter.consume() fails it means there is no more TokenType::Ops
|
// If limiter.consume() fails it means there is no more TokenType::Ops
|
||||||
// budget and rate limiting is in effect.
|
// budget and rate limiting is in effect.
|
||||||
@ -390,6 +412,7 @@ pub struct Block {
|
|||||||
seccomp_action: SeccompAction,
|
seccomp_action: SeccompAction,
|
||||||
rate_limiter_config: Option<RateLimiterConfig>,
|
rate_limiter_config: Option<RateLimiterConfig>,
|
||||||
exit_evt: EventFd,
|
exit_evt: EventFd,
|
||||||
|
read_only: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Versionize)]
|
#[derive(Versionize)]
|
||||||
@ -410,7 +433,7 @@ impl Block {
|
|||||||
id: String,
|
id: String,
|
||||||
mut disk_image: Box<dyn DiskFile>,
|
mut disk_image: Box<dyn DiskFile>,
|
||||||
disk_path: PathBuf,
|
disk_path: PathBuf,
|
||||||
is_disk_read_only: bool,
|
read_only: bool,
|
||||||
iommu: bool,
|
iommu: bool,
|
||||||
num_queues: usize,
|
num_queues: usize,
|
||||||
queue_size: u16,
|
queue_size: u16,
|
||||||
@ -452,7 +475,7 @@ impl Block {
|
|||||||
avail_features |= 1u64 << VIRTIO_F_IOMMU_PLATFORM;
|
avail_features |= 1u64 << VIRTIO_F_IOMMU_PLATFORM;
|
||||||
}
|
}
|
||||||
|
|
||||||
if is_disk_read_only {
|
if read_only {
|
||||||
avail_features |= 1u64 << VIRTIO_BLK_F_RO;
|
avail_features |= 1u64 << VIRTIO_BLK_F_RO;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -512,6 +535,7 @@ impl Block {
|
|||||||
seccomp_action,
|
seccomp_action,
|
||||||
rate_limiter_config,
|
rate_limiter_config,
|
||||||
exit_evt,
|
exit_evt,
|
||||||
|
read_only,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -644,6 +668,7 @@ impl VirtioDevice for Block {
|
|||||||
request_list: HashMap::with_capacity(queue_size.into()),
|
request_list: HashMap::with_capacity(queue_size.into()),
|
||||||
rate_limiter,
|
rate_limiter,
|
||||||
access_platform: self.common.access_platform.clone(),
|
access_platform: self.common.access_platform.clone(),
|
||||||
|
read_only: self.read_only,
|
||||||
};
|
};
|
||||||
|
|
||||||
let paused = self.common.paused.clone();
|
let paused = self.common.paused.clone();
|
||||||
|
Loading…
x
Reference in New Issue
Block a user