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

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@novell.com>

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




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