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