Commit 713b15b3 authored by Rusty Russell's avatar Rusty Russell

lguest: be paranoid about guest playing with device descriptors.

We can't trust the values in the device descriptor table once the
guest has booted, so keep local copies.  They could set them to
strange values then cause us to segv (they're 8 bit values, so they
can't make our pointers go too wild).

This becomes more important with the following patches which read them.
Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent 8ebf9756
...@@ -126,9 +126,13 @@ struct device ...@@ -126,9 +126,13 @@ struct device
/* The linked-list pointer. */ /* The linked-list pointer. */
struct device *next; struct device *next;
/* The this device's descriptor, as mapped into the Guest. */ /* The device's descriptor, as mapped into the Guest. */
struct lguest_device_desc *desc; struct lguest_device_desc *desc;
/* We can't trust desc values once Guest has booted: we use these. */
unsigned int feature_len;
unsigned int num_vq;
/* The name of this device, for --verbose. */ /* The name of this device, for --verbose. */
const char *name; const char *name;
...@@ -245,7 +249,7 @@ static void iov_consume(struct iovec iov[], unsigned num_iov, unsigned len) ...@@ -245,7 +249,7 @@ static void iov_consume(struct iovec iov[], unsigned num_iov, unsigned len)
static u8 *get_feature_bits(struct device *dev) static u8 *get_feature_bits(struct device *dev)
{ {
return (u8 *)(dev->desc + 1) return (u8 *)(dev->desc + 1)
+ dev->desc->num_vq * sizeof(struct lguest_vqconfig); + dev->num_vq * sizeof(struct lguest_vqconfig);
} }
/*L:100 The Launcher code itself takes us out into userspace, that scary place /*L:100 The Launcher code itself takes us out into userspace, that scary place
...@@ -979,8 +983,8 @@ static void update_device_status(struct device *dev) ...@@ -979,8 +983,8 @@ static void update_device_status(struct device *dev)
verbose("Resetting device %s\n", dev->name); verbose("Resetting device %s\n", dev->name);
/* Clear any features they've acked. */ /* Clear any features they've acked. */
memset(get_feature_bits(dev) + dev->desc->feature_len, 0, memset(get_feature_bits(dev) + dev->feature_len, 0,
dev->desc->feature_len); dev->feature_len);
/* Zero out the virtqueues. */ /* Zero out the virtqueues. */
for (vq = dev->vq; vq; vq = vq->next) { for (vq = dev->vq; vq; vq = vq->next) {
...@@ -994,12 +998,12 @@ static void update_device_status(struct device *dev) ...@@ -994,12 +998,12 @@ static void update_device_status(struct device *dev)
unsigned int i; unsigned int i;
verbose("Device %s OK: offered", dev->name); verbose("Device %s OK: offered", dev->name);
for (i = 0; i < dev->desc->feature_len; i++) for (i = 0; i < dev->feature_len; i++)
verbose(" %02x", get_feature_bits(dev)[i]); verbose(" %02x", get_feature_bits(dev)[i]);
verbose(", accepted"); verbose(", accepted");
for (i = 0; i < dev->desc->feature_len; i++) for (i = 0; i < dev->feature_len; i++)
verbose(" %02x", get_feature_bits(dev) verbose(" %02x", get_feature_bits(dev)
[dev->desc->feature_len+i]); [dev->feature_len+i]);
if (dev->ready) if (dev->ready)
dev->ready(dev); dev->ready(dev);
...@@ -1129,8 +1133,8 @@ static void handle_input(int fd) ...@@ -1129,8 +1133,8 @@ static void handle_input(int fd)
static u8 *device_config(const struct device *dev) static u8 *device_config(const struct device *dev)
{ {
return (void *)(dev->desc + 1) return (void *)(dev->desc + 1)
+ dev->desc->num_vq * sizeof(struct lguest_vqconfig) + dev->num_vq * sizeof(struct lguest_vqconfig)
+ dev->desc->feature_len * 2; + dev->feature_len * 2;
} }
/* This routine allocates a new "struct lguest_device_desc" from descriptor /* This routine allocates a new "struct lguest_device_desc" from descriptor
...@@ -1191,6 +1195,7 @@ static void add_virtqueue(struct device *dev, unsigned int num_descs, ...@@ -1191,6 +1195,7 @@ static void add_virtqueue(struct device *dev, unsigned int num_descs,
* yet, otherwise we'd be overwriting them. */ * yet, otherwise we'd be overwriting them. */
assert(dev->desc->config_len == 0 && dev->desc->feature_len == 0); assert(dev->desc->config_len == 0 && dev->desc->feature_len == 0);
memcpy(device_config(dev), &vq->config, sizeof(vq->config)); memcpy(device_config(dev), &vq->config, sizeof(vq->config));
dev->num_vq++;
dev->desc->num_vq++; dev->desc->num_vq++;
verbose("Virtqueue page %#lx\n", to_guest_phys(p)); verbose("Virtqueue page %#lx\n", to_guest_phys(p));
...@@ -1219,7 +1224,7 @@ static void add_feature(struct device *dev, unsigned bit) ...@@ -1219,7 +1224,7 @@ static void add_feature(struct device *dev, unsigned bit)
/* We can't extend the feature bits once we've added config bytes */ /* We can't extend the feature bits once we've added config bytes */
if (dev->desc->feature_len <= bit / CHAR_BIT) { if (dev->desc->feature_len <= bit / CHAR_BIT) {
assert(dev->desc->config_len == 0); assert(dev->desc->config_len == 0);
dev->desc->feature_len = (bit / CHAR_BIT) + 1; dev->feature_len = dev->desc->feature_len = (bit/CHAR_BIT) + 1;
} }
features[bit / CHAR_BIT] |= (1 << (bit % CHAR_BIT)); features[bit / CHAR_BIT] |= (1 << (bit % CHAR_BIT));
...@@ -1259,6 +1264,8 @@ static struct device *new_device(const char *name, u16 type, int fd, ...@@ -1259,6 +1264,8 @@ static struct device *new_device(const char *name, u16 type, int fd,
dev->name = name; dev->name = name;
dev->vq = NULL; dev->vq = NULL;
dev->ready = NULL; dev->ready = NULL;
dev->feature_len = 0;
dev->num_vq = 0;
/* Append to device list. Prepending to a single-linked list is /* Append to device list. Prepending to a single-linked list is
* easier, but the user expects the devices to be arranged on the bus * easier, but the user expects the devices to be arranged on the bus
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment