[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 |