[RFC PATCH 4/5] lguest: ignore bad virtqueues.

Rusty Russell
2008-03-20T06:40:28Z

Currently the lguest Launcher aborts when a Guest puts something bogus
in a virtio queue. If we want to deal with other (untrusted) Guests'
queues, that's a bad idea: simply print a warning and ignore it from
now on.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
---
Documentation/lguest/lguest.c | 45 +++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)

diff -r 784299890d4a Documentation/lguest/lguest.c
--- a/Documentation/lguest/lguest.c Thu Mar 13 23:10:14 2008 +1100
+++ b/Documentation/lguest/lguest.c Thu Mar 13 23:21:55 2008 +1100
@@ -156,6 +156,9 @@ struct virtqueue_info

/* Last available index we saw. */
u16 last_avail_idx;
+
+ /* Are we broken? If so, ignore it from now on. */
+ bool broken;
};

struct virtqueue
@@ -676,8 +679,11 @@ static unsigned next_desc(struct virtque
/* Make sure compiler knows to grab that: we don't want it changing! */
wmb();

- if (next >= vqi->vring.num)
- errx(1, "Desc next is %u", next);
+ if (next >= vqi->vring.num) {
+ warnx("Desc next is %u", next);
+ vqi->broken = true;
+ return vqi->vring.num;
+ }

return next;
}
@@ -695,10 +701,16 @@ static int get_vq_desc(struct virtqueue_
{
unsigned int i, head;

+ /* If the queue is broken, we just pretend there's nothing there. */
+ if (vqi->broken)
+ return -1;
+
/* Check it isn't doing very strange things with descriptor numbers. */
- if ((u16)(vqi->vring.avail->idx - vqi->last_avail_idx) > vqi->vring.num)
- errx(1, "Guest moved used index from %u to %u",
- vqi->last_avail_idx, vqi->vring.avail->idx);
+ if ((u16)(vqi->vring.avail->idx-vqi->last_avail_idx) > vqi->vring.num) {
+ warnx("Guest moved used index from %u to %u",
+ vqi->last_avail_idx, vqi->vring.avail->idx);
+ goto broken;
+ }

/* If there's nothing new since last we looked, return invalid. */
if (vqi->vring.avail->idx == vqi->last_avail_idx)
@@ -709,8 +721,10 @@ static int get_vq_desc(struct virtqueue_
head = vqi->vring.avail->ring[vqi->last_avail_idx++ % vqi->vring.num];

/* If their number is silly, that's a fatal mistake. */
- if (head >= vqi->vring.num)
- errx(1, "Guest says index %u is available", head);
+ if (head >= vqi->vring.num) {
+ warnx("Guest says index %u is available", head);
+ goto broken;
+ }

/* When we start there are none of either input nor output. */
*out_num = *in_num = 0;
@@ -728,17 +742,25 @@ static int get_vq_desc(struct virtqueue_
else {
/* If it's an output descriptor, they're all supposed
* to come before any input descriptors. */
- if (*in_num)
- errx(1, "Descriptor has out after in");
+ if (*in_num) {
+ warnx("Descriptor has out after in");
+ goto broken;
+ }
(*out_num)++;
}

/* If we've got too many, that implies a descriptor loop. */
- if (*out_num + *in_num > vqi->vring.num)
- errx(1, "Looped descriptor");
+ if (*out_num + *in_num > vqi->vring.num) {
+ warnx("Looped descriptor");
+ goto broken;
+ }
} while ((i = next_desc(vqi, i)) != vqi->vring.num);

return head;
+
+broken:
+ vqi->broken = true;
+ return -1;
}

/* After we've used one of their buffers, we tell them about it. We'll then
@@ -1127,6 +1149,7 @@ static void add_virtqueue(struct device
vq->dev = dev;
vq->vqi.last_avail_idx = 0;
vq->vqi.mem = &gmem;
+ vq->vqi.broken = false;

/* Initialize the configuration. */
vq->config.num = num_descs;