virtio-blk: undo destructive iov_discard_*() operations
Fuzzing discovered that virtqueue_unmap_sg() is being called on modified
req->in/out_sg iovecs. This means dma_memory_map() and
dma_memory_unmap() calls do not have matching memory addresses.
Fuzzing discovered that non-RAM addresses trigger a bug:
void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
bool is_write, hwaddr access_len)
{
if (buffer != bounce.buffer) {
^^^^^^^^^^^^^^^^^^^^^^^
A modified iov->iov_base is no longer recognized as a bounce buffer and
the wrong branch is taken.
There are more potential bugs: dirty memory is not tracked correctly and
MemoryRegion refcounts can be leaked.
Use the new iov_discard_undo() API to restore elem->in/out_sg before
virtqueue_push() is called.
Fixes: 827805a249
("virtio-blk: Convert VirtIOBlockReq.out to structrue")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Buglink: https://bugs.launchpad.net/qemu/+bug/1890360
Message-Id: <20200917094455.822379-3-stefanha@redhat.com>
This commit is contained in:
parent
9dd6f7c28e
commit
7bd04a041a
|
@ -80,6 +80,8 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status)
|
||||||
trace_virtio_blk_req_complete(vdev, req, status);
|
trace_virtio_blk_req_complete(vdev, req, status);
|
||||||
|
|
||||||
stb_p(&req->in->status, status);
|
stb_p(&req->in->status, status);
|
||||||
|
iov_discard_undo(&req->inhdr_undo);
|
||||||
|
iov_discard_undo(&req->outhdr_undo);
|
||||||
virtqueue_push(req->vq, &req->elem, req->in_len);
|
virtqueue_push(req->vq, &req->elem, req->in_len);
|
||||||
if (s->dataplane_started && !s->dataplane_disabled) {
|
if (s->dataplane_started && !s->dataplane_disabled) {
|
||||||
virtio_blk_data_plane_notify(s->dataplane, req->vq);
|
virtio_blk_data_plane_notify(s->dataplane, req->vq);
|
||||||
|
@ -632,10 +634,12 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
iov_discard_front(&out_iov, &out_num, sizeof(req->out));
|
iov_discard_front_undoable(&out_iov, &out_num, sizeof(req->out),
|
||||||
|
&req->outhdr_undo);
|
||||||
|
|
||||||
if (in_iov[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) {
|
if (in_iov[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) {
|
||||||
virtio_error(vdev, "virtio-blk request inhdr too short");
|
virtio_error(vdev, "virtio-blk request inhdr too short");
|
||||||
|
iov_discard_undo(&req->outhdr_undo);
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -644,7 +648,8 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
|
||||||
req->in = (void *)in_iov[in_num - 1].iov_base
|
req->in = (void *)in_iov[in_num - 1].iov_base
|
||||||
+ in_iov[in_num - 1].iov_len
|
+ in_iov[in_num - 1].iov_len
|
||||||
- sizeof(struct virtio_blk_inhdr);
|
- sizeof(struct virtio_blk_inhdr);
|
||||||
iov_discard_back(in_iov, &in_num, sizeof(struct virtio_blk_inhdr));
|
iov_discard_back_undoable(in_iov, &in_num, sizeof(struct virtio_blk_inhdr),
|
||||||
|
&req->inhdr_undo);
|
||||||
|
|
||||||
type = virtio_ldl_p(vdev, &req->out.type);
|
type = virtio_ldl_p(vdev, &req->out.type);
|
||||||
|
|
||||||
|
@ -739,6 +744,8 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
|
||||||
|
|
||||||
if (unlikely(iov_to_buf(out_iov, out_num, 0, &dwz_hdr,
|
if (unlikely(iov_to_buf(out_iov, out_num, 0, &dwz_hdr,
|
||||||
sizeof(dwz_hdr)) != sizeof(dwz_hdr))) {
|
sizeof(dwz_hdr)) != sizeof(dwz_hdr))) {
|
||||||
|
iov_discard_undo(&req->inhdr_undo);
|
||||||
|
iov_discard_undo(&req->outhdr_undo);
|
||||||
virtio_error(vdev, "virtio-blk discard/write_zeroes header"
|
virtio_error(vdev, "virtio-blk discard/write_zeroes header"
|
||||||
" too short");
|
" too short");
|
||||||
return -1;
|
return -1;
|
||||||
|
|
|
@ -70,6 +70,8 @@ typedef struct VirtIOBlockReq {
|
||||||
int64_t sector_num;
|
int64_t sector_num;
|
||||||
VirtIOBlock *dev;
|
VirtIOBlock *dev;
|
||||||
VirtQueue *vq;
|
VirtQueue *vq;
|
||||||
|
IOVDiscardUndo inhdr_undo;
|
||||||
|
IOVDiscardUndo outhdr_undo;
|
||||||
struct virtio_blk_inhdr *in;
|
struct virtio_blk_inhdr *in;
|
||||||
struct virtio_blk_outhdr out;
|
struct virtio_blk_outhdr out;
|
||||||
QEMUIOVector qiov;
|
QEMUIOVector qiov;
|
||||||
|
|
Loading…
Reference in a new issue