WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] linux-2.6.18/xenbus: don't BUG() on user mode induced conditions
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Wed, 01 Jun 2011 08:11:42 +0100
Delivery-date: Wed, 01 Jun 2011 00:14:49 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] linux-2.6.18/xenbus: don't BUG() on user mode induced conditions, Jan Beulich <=