[Xen-devel] [PATCH] linux-2.6.18/xenbus: don't BUG() on user mode induced conditions

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

Neither allocation failure nor inability to locate a user mode
specified transaction ID should lead to a kernel crash. For
XS_TRANSACTION_END also don't issue anything to xenbus if the
specified ID doesn't match that of any active transaction.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>

--- a/drivers/xen/xenbus/xenbus_dev.c
+++ b/drivers/xen/xenbus/xenbus_dev.c
@@ -133,25 +133,42 @@ static ssize_t xenbus_dev_read(struct fi
        return i;
 }
 
-static void queue_reply(struct xenbus_dev_data *u,
-                       char *data, unsigned int len)
+static int queue_reply(struct list_head *queue,
+                      const void *data, unsigned int len)
 {
        struct read_buffer *rb;
 
        if (len == 0)
-               return;
+               return 0;
 
        rb = kmalloc(sizeof(*rb) + len, GFP_KERNEL);
-       BUG_ON(rb == NULL);
+       if (!rb)
+               return -ENOMEM;
 
        rb->cons = 0;
        rb->len = len;
 
        memcpy(rb->msg, data, len);
 
-       list_add_tail(&rb->list, &u->read_buffers);
+       list_add_tail(&rb->list, queue);
+       return 0;
+}
+
+static void queue_flush(struct xenbus_dev_data *u, struct list_head *queue,
+                       int err)
+{
+       if (!err) {
+               /* list_splice_tail(queue, &u->read_buffers); */
+               list_splice(queue, u->read_buffers.prev);
+               wake_up(&u->read_waitq);
+       } else
+               while (!list_empty(queue)) {
+                       struct read_buffer *rb = list_entry(queue->next,
+                               struct read_buffer, list);
 
-       wake_up(&u->read_waitq);
+                       list_del(queue->next);
+                       kfree(rb);
+               }
 }
 
 struct watch_adapter
@@ -177,8 +194,9 @@ static void watch_fired(struct xenbus_wa
             container_of(watch, struct watch_adapter, watch);
        struct xsd_sockmsg hdr;
        const char *path, *token;
-       int path_len, tok_len, body_len, data_len = 0;
-       
+       int err, path_len, tok_len, body_len, data_len = 0;
+       LIST_HEAD(queue);
+
        path = vec[XS_WATCH_PATH];
        token = adap->token;
 
@@ -192,11 +210,14 @@ static void watch_fired(struct xenbus_wa
        hdr.len = body_len;
 
        mutex_lock(&adap->dev_data->reply_mutex);
-       queue_reply(adap->dev_data, (char *)&hdr, sizeof(hdr));
-       queue_reply(adap->dev_data, (char *)path, path_len);
-       queue_reply(adap->dev_data, (char *)token, tok_len);
-       if (len > 2)
-               queue_reply(adap->dev_data, (char *)vec[2], data_len);
+       err = queue_reply(&queue, &hdr, sizeof(hdr));
+       if (!err)
+               err = queue_reply(&queue, path, path_len);
+       if (!err)
+               err = queue_reply(&queue, token, tok_len);
+       if (!err && len > 2)
+               err = queue_reply(&queue, vec[2], data_len);
+       queue_flush(adap->dev_data, &queue, err);
        mutex_unlock(&adap->dev_data->reply_mutex);
 }
 
@@ -209,7 +230,8 @@ static ssize_t xenbus_dev_write(struct f
        struct xenbus_dev_data *u = filp->private_data;
        struct xenbus_dev_transaction *trans = NULL;
        uint32_t msg_type;
-       void *reply;
+       void *reply = NULL;
+       LIST_HEAD(queue);
        char *path, *token;
        struct watch_adapter *watch, *tmp_watch;
        int err, rc = len;
@@ -237,7 +259,7 @@ static ssize_t xenbus_dev_write(struct f
        switch (msg_type) {
        case XS_WATCH:
        case XS_UNWATCH: {
-               static const char *XS_RESP = "OK";
+               static const char XS_RESP[] = "OK";
                struct xsd_sockmsg hdr;
 
                path = u->u.buffer + sizeof(u->u.msg);
@@ -281,26 +303,36 @@ static ssize_t xenbus_dev_write(struct f
                }
 
                hdr.type = msg_type;
-               hdr.len = strlen(XS_RESP) + 1;
+               hdr.len = sizeof(XS_RESP);
                mutex_lock(&u->reply_mutex);
-               queue_reply(u, (char *)&hdr, sizeof(hdr));
-               queue_reply(u, (char *)XS_RESP, hdr.len);
-               mutex_unlock(&u->reply_mutex);
+               err = queue_reply(&queue, &hdr, sizeof(hdr))
+                     ?: queue_reply(&queue, XS_RESP, hdr.len);
                break;
        }
 
-       default:
-               if (msg_type == XS_TRANSACTION_START) {
-                       trans = kmalloc(sizeof(*trans), GFP_KERNEL);
-                       if (!trans) {
-                               rc = -ENOMEM;
-                               goto out;
-                       }
+       case XS_TRANSACTION_START:
+               trans = kmalloc(sizeof(*trans), GFP_KERNEL);
+               if (!trans) {
+                       rc = -ENOMEM;
+                       goto out;
                }
+               goto common;
 
+       case XS_TRANSACTION_END:
+               list_for_each_entry(trans, &u->transactions, list)
+                       if (trans->handle.id == u->u.msg.tx_id)
+                               break;
+               if (&trans->list == &u->transactions) {
+                       rc = -ESRCH;
+                       goto out;
+               }
+               /* fall through */
+       common:
+       default:
                reply = xenbus_dev_request_and_reply(&u->u.msg);
                if (IS_ERR(reply)) {
-                       kfree(trans);
+                       if (msg_type == XS_TRANSACTION_START)
+                               kfree(trans);
                        rc = PTR_ERR(reply);
                        goto out;
                }
@@ -309,21 +341,21 @@ static ssize_t xenbus_dev_write(struct f
                        trans->handle.id = simple_strtoul(reply, NULL, 0);
                        list_add(&trans->list, &u->transactions);
                } else if (msg_type == XS_TRANSACTION_END) {
-                       list_for_each_entry(trans, &u->transactions, list)
-                               if (trans->handle.id == u->u.msg.tx_id)
-                                       break;
-                       BUG_ON(&trans->list == &u->transactions);
                        list_del(&trans->list);
                        kfree(trans);
                }
                mutex_lock(&u->reply_mutex);
-               queue_reply(u, (char *)&u->u.msg, sizeof(u->u.msg));
-               queue_reply(u, (char *)reply, u->u.msg.len);
-               mutex_unlock(&u->reply_mutex);
-               kfree(reply);
+               err = queue_reply(&queue, &u->u.msg, sizeof(u->u.msg))
+                     ?: queue_reply(&queue, reply, u->u.msg.len);
                break;
        }
 
+       queue_flush(u, &queue, err);
+       mutex_unlock(&u->reply_mutex);
+       kfree(reply);
+       if (err)
+               rc = err;
+
  out:
        u->len = 0;
        return rc;

Attachment: xen-xenbus-dev-no-BUG.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel