|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] Prevent unnatural use of ioctl in /proc/xen/xenbus_dev
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
# HG changeset patch
# User Rusty Russell <rusty@xxxxxxxxxxxxxxx>
# Node ID df3759bbb309f6d01d6087af06ba4297a5169538
# Parent f0d728001aaad4eb6c716cbdbb5d1f8a8a5f1620
xenbus_dev's use of ioctl for read/write is a crime against nature.
Make it a read-write interface, but check boundaries so we can recover if
userspace dies. This also simplifies libxenstore.
diff -r f0d728001aaa -r df3759bbb309
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_dev.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_dev.c Wed Sep 7
23:11:44 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_dev.c Fri Sep 9
11:07:29 2005
@@ -5,6 +5,7 @@
* to xenstore.
*
* Copyright (c) 2005, Christian Limpach
+ * Copyright (c) 2005, Rusty Russell, IBM Corporation
*
* This file may be distributed separately from the Linux kernel, or
* incorporated into other software packages, subject to the following license:
@@ -45,100 +46,93 @@
#include <asm-xen/xen_proc.h>
struct xenbus_dev_data {
- int in_transaction;
+ /* Are there bytes left to be read in this message? */
+ int bytes_left;
+ /* Are we still waiting for the reply to a message we wrote? */
+ int awaiting_reply;
+ /* Buffer for outgoing messages. */
+ unsigned int len;
+ union {
+ struct xsd_sockmsg msg;
+ char buffer[PAGE_SIZE];
+ } u;
};
static struct proc_dir_entry *xenbus_dev_intf;
-void *xs_talkv(enum xsd_sockmsg_type type, const struct kvec *iovec,
- unsigned int num_vecs, unsigned int *len);
+/* Reply can be long (dir, getperm): don't buffer, just examine
+ * headers so we can discard rest if they die. */
+static ssize_t xenbus_dev_read(struct file *filp,
+ char __user *ubuf,
+ size_t len, loff_t *ppos)
+{
+ struct xenbus_dev_data *data = filp->private_data;
+ struct xsd_sockmsg msg;
+ int err;
-static int xenbus_dev_talkv(struct xenbus_dev_data *u, unsigned long data)
-{
- struct xenbus_dev_talkv xt;
- unsigned int len;
- void *resp, *base;
- struct kvec *iovec;
- int ret = -EFAULT, v = 0;
+ /* Refill empty buffer? */
+ if (data->bytes_left == 0) {
+ if (len < sizeof(msg))
+ return -EINVAL;
- if (copy_from_user(&xt, (void *)data, sizeof(xt)))
- return -EFAULT;
-
- iovec = kmalloc(xt.num_vecs * sizeof(struct kvec), GFP_KERNEL);
- if (iovec == NULL)
- return -ENOMEM;
-
- if (copy_from_user(iovec, xt.iovec,
- xt.num_vecs * sizeof(struct kvec)))
- goto out;
-
- for (v = 0; v < xt.num_vecs; v++) {
- base = iovec[v].iov_base;
- iovec[v].iov_base = kmalloc(iovec[v].iov_len, GFP_KERNEL);
- if (iovec[v].iov_base == NULL ||
- copy_from_user(iovec[v].iov_base, base, iovec[v].iov_len))
- {
- if (iovec[v].iov_base)
- kfree(iovec[v].iov_base);
- else
- ret = -ENOMEM;
- v--;
- goto out;
- }
+ err = xb_read(&msg, sizeof(msg));
+ if (err)
+ return err;
+ data->bytes_left = msg.len;
+ if (ubuf && copy_to_user(ubuf, &msg, sizeof(msg)) != 0)
+ return -EFAULT;
+ /* We can receive spurious XS_WATCH_EVENT messages. */
+ if (msg.type != XS_WATCH_EVENT)
+ data->awaiting_reply = 0;
+ return sizeof(msg);
}
- resp = xs_talkv(xt.type, iovec, xt.num_vecs, &len);
- if (IS_ERR(resp)) {
- ret = PTR_ERR(resp);
- goto out;
- }
+ /* Don't read over next header, or over temporary buffer. */
+ if (len > sizeof(data->u.buffer))
+ len = sizeof(data->u.buffer);
+ if (len > data->bytes_left)
+ len = data->bytes_left;
- switch (xt.type) {
- case XS_TRANSACTION_START:
- u->in_transaction = 1;
- break;
- case XS_TRANSACTION_END:
- u->in_transaction = 0;
- break;
- default:
- break;
- }
+ err = xb_read(data->u.buffer, len);
+ if (err)
+ return err;
- ret = len;
- if (len > xt.len)
- len = xt.len;
-
- if (copy_to_user(xt.buf, resp, len))
- ret = -EFAULT;
-
- kfree(resp);
- out:
- while (v-- > 0)
- kfree(iovec[v].iov_base);
- kfree(iovec);
- return ret;
+ data->bytes_left -= len;
+ if (ubuf && copy_to_user(ubuf, data->u.buffer, len) != 0)
+ return -EFAULT;
+ return len;
}
-static int xenbus_dev_ioctl(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long data)
+/* We do v. basic sanity checking so they don't screw up kernel later. */
+static ssize_t xenbus_dev_write(struct file *filp,
+ const char __user *ubuf,
+ size_t len, loff_t *ppos)
{
- struct xenbus_dev_data *u = filp->private_data;
- int ret = -ENOSYS;
+ struct xenbus_dev_data *data = filp->private_data;
+ int err;
- switch (cmd) {
- case IOCTL_XENBUS_DEV_TALKV:
- ret = xenbus_dev_talkv(u, data);
- break;
- default:
- ret = -EINVAL;
- break;
+ /* We gather data in buffer until we're ready to send it. */
+ if (len > data->len + sizeof(data->u))
+ return -EINVAL;
+ if (copy_from_user(data->u.buffer + data->len, ubuf, len) != 0)
+ return -EFAULT;
+ data->len += len;
+ if (data->len >= sizeof(data->u.msg) + data->u.msg.len) {
+ err = xb_write(data->u.buffer, data->len);
+ if (err)
+ return err;
+ data->len = 0;
+ data->awaiting_reply = 1;
}
- return ret;
+ return len;
}
static int xenbus_dev_open(struct inode *inode, struct file *filp)
{
struct xenbus_dev_data *u;
+
+ /* Don't try seeking. */
+ nonseekable_open(inode, filp);
u = kmalloc(sizeof(*u), GFP_KERNEL);
if (u == NULL)
@@ -155,20 +149,25 @@
static int xenbus_dev_release(struct inode *inode, struct file *filp)
{
- struct xenbus_dev_data *u = filp->private_data;
+ struct xenbus_dev_data *data = filp->private_data;
- if (u->in_transaction)
- xenbus_transaction_end(1);
+ /* Discard any unread replies. */
+ while (data->bytes_left || data->awaiting_reply)
+ xenbus_dev_read(filp, NULL, sizeof(data->u.buffer), NULL);
+
+ /* Harmless if no transaction in progress. */
+ xenbus_transaction_end(1);
up(&xenbus_lock);
- kfree(u);
+ kfree(data);
return 0;
}
static struct file_operations xenbus_dev_file_ops = {
- ioctl: xenbus_dev_ioctl,
+ read: xenbus_dev_read,
+ write: xenbus_dev_write,
open: xenbus_dev_open,
release: xenbus_dev_release
};
diff -r f0d728001aaa -r df3759bbb309
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_xs.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_xs.c Wed Sep 7
23:11:44 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_xs.c Fri Sep 9
11:07:29 2005
@@ -106,10 +106,10 @@
}
/* Send message to xs, get kmalloc'ed reply. ERR_PTR() on error. */
-void *xs_talkv(enum xsd_sockmsg_type type,
- const struct kvec *iovec,
- unsigned int num_vecs,
- unsigned int *len)
+static void *xs_talkv(enum xsd_sockmsg_type type,
+ const struct kvec *iovec,
+ unsigned int num_vecs,
+ unsigned int *len)
{
struct xsd_sockmsg msg;
void *ret = NULL;
diff -r f0d728001aaa -r df3759bbb309 tools/xenstore/xs.c
--- a/tools/xenstore/xs.c Wed Sep 7 23:11:44 2005
+++ b/tools/xenstore/xs.c Fri Sep 9 11:07:29 2005
@@ -41,7 +41,6 @@
struct xs_handle
{
int fd;
- enum { SOCK, DEV } type;
};
/* Get the socket from the store daemon handle.
@@ -68,7 +67,6 @@
h = malloc(sizeof(*h));
if (h) {
h->fd = sock;
- h->type = SOCK;
return h;
}
}
@@ -82,16 +80,15 @@
static struct xs_handle *get_dev(const char *connect_to)
{
int fd, saved_errno;
- struct xs_handle *h = NULL;
-
- fd = open(connect_to, O_RDONLY);
+ struct xs_handle *h;
+
+ fd = open(connect_to, O_RDWR);
if (fd < 0)
return NULL;
h = malloc(sizeof(*h));
if (h) {
h->fd = fd;
- h->type = DEV;
return h;
}
@@ -190,9 +187,9 @@
}
/* Send message to xs, get malloc'ed reply. NULL and set errno on error. */
-static void *xs_talkv_sock(struct xs_handle *h, enum xsd_sockmsg_type type,
- const struct iovec *iovec, unsigned int num_vecs,
- unsigned int *len)
+static void *xs_talkv(struct xs_handle *h, enum xsd_sockmsg_type type,
+ const struct iovec *iovec, unsigned int num_vecs,
+ unsigned int *len)
{
struct xsd_sockmsg msg;
void *ret = NULL;
@@ -250,54 +247,6 @@
close(h->fd);
h->fd = -1;
errno = saved_errno;
- return NULL;
-}
-
-/* Send message to xs, get malloc'ed reply. NULL and set errno on error. */
-static void *xs_talkv_dev(struct xs_handle *h, enum xsd_sockmsg_type type,
- const struct iovec *iovec, unsigned int num_vecs,
- unsigned int *len)
-{
- struct xenbus_dev_talkv dt;
- char *buf;
- int err, buflen = 1024;
-
- again:
- buf = malloc(buflen);
- if (buf == NULL) {
- errno = ENOMEM;
- return NULL;
- }
- dt.type = type;
- dt.iovec = (struct kvec *)iovec;
- dt.num_vecs = num_vecs;
- dt.buf = buf;
- dt.len = buflen;
- err = ioctl(h->fd, IOCTL_XENBUS_DEV_TALKV, &dt);
- if (err < 0) {
- free(buf);
- errno = err;
- return NULL;
- }
- if (err > buflen) {
- free(buf);
- buflen = err;
- goto again;
- }
- if (len)
- *len = err;
- return buf;
-}
-
-/* Send message to xs, get malloc'ed reply. NULL and set errno on error. */
-static void *xs_talkv(struct xs_handle *h, enum xsd_sockmsg_type type,
- const struct iovec *iovec, unsigned int num_vecs,
- unsigned int *len)
-{
- if (h->type == SOCK)
- return xs_talkv_sock(h, type, iovec, num_vecs, len);
- if (h->type == DEV)
- return xs_talkv_dev(h, type, iovec, num_vecs, len);
return NULL;
}
--
A bad analogy is like a leaky screwdriver -- Richard Braakman
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |