vhost_blk_handle_guest_kick() declares the descriptor head as
u16 head;
and then does
head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
&out, &in, NULL, NULL);
if (unlikely(head < 0))
break;
if (unlikely(head == vq->num)) {
...
vhost_get_vq_desc() returns an int with three distinct outcomes:
- a negative errno (-EFAULT / -EINVAL) when the avail ring or a
descriptor is malformed,
- vq->num when there is nothing available right now,
- the descriptor head index (0 .. vq->num - 1) on success.
Storing that int into a u16 breaks the error case in two ways:
1. The negative errno is truncated to a large positive 16-bit value
(e.g. -EINVAL (-22) becomes 0xffea == 65514).
2. "head < 0" is therefore a comparison of an unsigned value against
zero, which is always false (the compiler folds it away under
-Wtype-limits). The error branch is dead code.
So on a vhost_get_vq_desc() failure we neither break nor hit the
"head == vq->num" branch (65514 does not equal the ring size). Control
falls through and the bogus value is used as an array index in
vhost_blk_req_handle():
req = &blk_vq->req[head]; /* blk_vq->req has vq->num entries */
blk_vq->req is allocated with vq->num elements, so indexing it with
~65514 is a wild out-of-bounds access tens of megabytes past the
allocation; the subsequent stores to req->blk_vq, req->head, etc.
corrupt unrelated kernel memory.
The bug is only reachable when vhost_get_vq_desc() actually returns an
error, i.e. when the guest presents a corrupted avail ring or descriptor
chain. Normal operation is unaffected because a valid head (0 ..
vq->num - 1) fits in a u16 and the "head == vq->num" empty-ring sentinel
also fits, which is why this has gone unnoticed.
Declare head as int. Then the "head < 0" error check works, the
"head == vq->num" comparison is unchanged (head is non-negative there),
and assigning it to req->head (u16) stays safe because a valid head is
always < vq->num <= 0xffff.
Fixes: 40a5928ec730 ("drivers/vhost: vhost-blk accelerator for virtio-blk
guests")
Feature: vhost-blk: in-kernel accelerator for virtio-blk guests
Signed-off-by: Konstantin Khorenko <[email protected]>
---
drivers/vhost/blk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/blk.c b/drivers/vhost/blk.c
index ed9b7041893f2..61de56cd73a0a 100644
--- a/drivers/vhost/blk.c
+++ b/drivers/vhost/blk.c
@@ -502,7 +502,7 @@ static void vhost_blk_handle_guest_kick(struct vhost_work
*work)
struct iov_iter iter;
int in, out, ret;
struct file *f;
- u16 head;
+ int head;
vq = container_of(work, struct vhost_virtqueue, poll.work);
blk = container_of(vq->dev, struct vhost_blk, dev);
--
2.43.0
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
