From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] linux-2.6.18/xenbus: don't BUG() on user mode induced conditions Date: Wed, 01 Jun 2011 08:11:42 +0100 Message-ID: <4DE6024E0200007800044BED@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part1834BD3E.0__=" Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__Part1834BD3E.0__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline 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; } =20 -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; =20 if (len =3D=3D 0) - return; + return 0; =20 rb =3D kmalloc(sizeof(*rb) + len, GFP_KERNEL); - BUG_ON(rb =3D=3D NULL); + if (!rb) + return -ENOMEM; =20 rb->cons =3D 0; rb->len =3D len; =20 memcpy(rb->msg, data, len); =20 - 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 =3D list_entry(queue->next, + struct read_buffer, list); =20 - wake_up(&u->read_waitq); + list_del(queue->next); + kfree(rb); + } } =20 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 =3D 0; -=09 + int err, path_len, tok_len, body_len, data_len =3D 0; + LIST_HEAD(queue); + path =3D vec[XS_WATCH_PATH]; token =3D adap->token; =20 @@ -192,11 +210,14 @@ static void watch_fired(struct xenbus_wa hdr.len =3D body_len; =20 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 =3D queue_reply(&queue, &hdr, sizeof(hdr)); + if (!err) + err =3D queue_reply(&queue, path, path_len); + if (!err) + err =3D queue_reply(&queue, token, tok_len); + if (!err && len > 2) + err =3D queue_reply(&queue, vec[2], data_len); + queue_flush(adap->dev_data, &queue, err); mutex_unlock(&adap->dev_data->reply_mutex); } =20 @@ -209,7 +230,8 @@ static ssize_t xenbus_dev_write(struct f struct xenbus_dev_data *u =3D filp->private_data; struct xenbus_dev_transaction *trans =3D NULL; uint32_t msg_type; - void *reply; + void *reply =3D NULL; + LIST_HEAD(queue); char *path, *token; struct watch_adapter *watch, *tmp_watch; int err, rc =3D 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 =3D "OK"; + static const char XS_RESP[] =3D "OK"; struct xsd_sockmsg hdr; =20 path =3D u->u.buffer + sizeof(u->u.msg); @@ -281,26 +303,36 @@ static ssize_t xenbus_dev_write(struct f } =20 hdr.type =3D msg_type; - hdr.len =3D strlen(XS_RESP) + 1; + hdr.len =3D 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 =3D queue_reply(&queue, &hdr, sizeof(hdr)) + ?: queue_reply(&queue, XS_RESP, hdr.len); break; } =20 - default: - if (msg_type =3D=3D XS_TRANSACTION_START) { - trans =3D kmalloc(sizeof(*trans), GFP_KERNEL); - if (!trans) { - rc =3D -ENOMEM; - goto out; - } + case XS_TRANSACTION_START: + trans =3D kmalloc(sizeof(*trans), GFP_KERNEL); + if (!trans) { + rc =3D -ENOMEM; + goto out; } + goto common; =20 + case XS_TRANSACTION_END: + list_for_each_entry(trans, &u->transactions, list) + if (trans->handle.id =3D=3D u->u.msg.tx_id) + break; + if (&trans->list =3D=3D &u->transactions) { + rc =3D -ESRCH; + goto out; + } + /* fall through */ + common: + default: reply =3D xenbus_dev_request_and_reply(&u->u.msg); if (IS_ERR(reply)) { - kfree(trans); + if (msg_type =3D=3D XS_TRANSACTION_START) + kfree(trans); rc =3D PTR_ERR(reply); goto out; } @@ -309,21 +341,21 @@ static ssize_t xenbus_dev_write(struct f trans->handle.id =3D simple_strtoul(reply, NULL, = 0); list_add(&trans->list, &u->transactions); } else if (msg_type =3D=3D XS_TRANSACTION_END) { - list_for_each_entry(trans, &u->transactions, list) - if (trans->handle.id =3D=3D u->u.msg.tx_id)= - break; - BUG_ON(&trans->list =3D=3D &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 =3D queue_reply(&queue, &u->u.msg, sizeof(u->u.msg)) + ?: queue_reply(&queue, reply, u->u.msg.len); break; } =20 + queue_flush(u, &queue, err); + mutex_unlock(&u->reply_mutex); + kfree(reply); + if (err) + rc =3D err; + out: u->len =3D 0; return rc; --=__Part1834BD3E.0__= Content-Type: text/plain; name="xen-xenbus-dev-no-BUG.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="xen-xenbus-dev-no-BUG.patch" Subject: xenbus: don't BUG() on user mode induced conditions=0A=0ANeither = allocation failure nor inability to locate a user mode=0Aspecified = transaction ID should lead to a kernel crash. For=0AXS_TRANSACTION_END = also don't issue anything to xenbus if the=0Aspecified ID doesn't match = that of any active transaction.=0A=0ASigned-off-by: Jan Beulich =0A=0A--- a/drivers/xen/xenbus/xenbus_dev.c=0A+++ b/drivers/xen/x= enbus/xenbus_dev.c=0A@@ -133,25 +133,42 @@ static ssize_t xenbus_dev_read(s= truct fi=0A return i;=0A }=0A =0A-static void queue_reply(struct = xenbus_dev_data *u,=0A- char *data, unsigned int len)=0A+st= atic int queue_reply(struct list_head *queue,=0A+ = const void *data, unsigned int len)=0A {=0A struct read_buffer *rb;=0A = =0A if (len =3D=3D 0)=0A- return;=0A+ return = 0;=0A =0A rb =3D kmalloc(sizeof(*rb) + len, GFP_KERNEL);=0A- = BUG_ON(rb =3D=3D NULL);=0A+ if (!rb)=0A+ return -ENOMEM;=0A = =0A rb->cons =3D 0;=0A rb->len =3D len;=0A =0A memcpy(rb->= msg, data, len);=0A =0A- list_add_tail(&rb->list, &u->read_buffers);= =0A+ list_add_tail(&rb->list, queue);=0A+ return 0;=0A+}=0A+=0A+stati= c void queue_flush(struct xenbus_dev_data *u, struct list_head *queue,=0A+ = int err)=0A+{=0A+ if (!err) {=0A+ /* = list_splice_tail(queue, &u->read_buffers); */=0A+ list_splice= (queue, u->read_buffers.prev);=0A+ wake_up(&u->read_waitq);=0A= + } else=0A+ while (!list_empty(queue)) {=0A+ = struct read_buffer *rb =3D list_entry(queue->next,=0A+ = struct read_buffer, list);=0A =0A- wake_up(&u->read_waitq);=0A= + list_del(queue->next);=0A+ = kfree(rb);=0A+ }=0A }=0A =0A struct watch_adapter=0A@@ -177,8 = +194,9 @@ static void watch_fired(struct xenbus_wa=0A = container_of(watch, struct watch_adapter, watch);=0A struct xsd_sockmsg = hdr;=0A const char *path, *token;=0A- int path_len, tok_len, = body_len, data_len =3D 0;=0A- =0A+ int err, path_len, tok_len, = body_len, data_len =3D 0;=0A+ LIST_HEAD(queue);=0A+=0A path =3D = vec[XS_WATCH_PATH];=0A token =3D adap->token;=0A =0A@@ -192,11 +210,14 @@ = static void watch_fired(struct xenbus_wa=0A hdr.len =3D body_len;=0A = =0A mutex_lock(&adap->dev_data->reply_mutex);=0A- queue_reply(adap->d= ev_data, (char *)&hdr, sizeof(hdr));=0A- queue_reply(adap->dev_data,= (char *)path, path_len);=0A- queue_reply(adap->dev_data, (char *)token, = tok_len);=0A- if (len > 2)=0A- queue_reply(adap->dev_data,= (char *)vec[2], data_len);=0A+ err =3D queue_reply(&queue, &hdr, = sizeof(hdr));=0A+ if (!err)=0A+ err =3D queue_reply(&queue,= path, path_len);=0A+ if (!err)=0A+ err =3D queue_reply(&queue,= token, tok_len);=0A+ if (!err && len > 2)=0A+ err =3D = queue_reply(&queue, vec[2], data_len);=0A+ queue_flush(adap->dev_data,= &queue, err);=0A mutex_unlock(&adap->dev_data->reply_mutex);=0A = }=0A =0A@@ -209,7 +230,8 @@ static ssize_t xenbus_dev_write(struct f=0A = struct xenbus_dev_data *u =3D filp->private_data;=0A struct xenbus_dev_t= ransaction *trans =3D NULL;=0A uint32_t msg_type;=0A- void *reply;=0A+ = void *reply =3D NULL;=0A+ LIST_HEAD(queue);=0A char *path, = *token;=0A struct watch_adapter *watch, *tmp_watch;=0A int err, = rc =3D len;=0A@@ -237,7 +259,7 @@ static ssize_t xenbus_dev_write(struct = f=0A switch (msg_type) {=0A case XS_WATCH:=0A case XS_UNWATCH: = {=0A- static const char *XS_RESP =3D "OK";=0A+ = static const char XS_RESP[] =3D "OK";=0A struct xsd_sockmsg = hdr;=0A =0A path =3D u->u.buffer + sizeof(u->u.msg);=0A@@ = -281,26 +303,36 @@ static ssize_t xenbus_dev_write(struct f=0A = }=0A =0A hdr.type =3D msg_type;=0A- hdr.len = =3D strlen(XS_RESP) + 1;=0A+ hdr.len =3D sizeof(XS_RESP);=0A = mutex_lock(&u->reply_mutex);=0A- queue_reply(u, = (char *)&hdr, sizeof(hdr));=0A- queue_reply(u, (char *)XS_RESP, = hdr.len);=0A- mutex_unlock(&u->reply_mutex);=0A+ = err =3D queue_reply(&queue, &hdr, sizeof(hdr))=0A+ ?: = queue_reply(&queue, XS_RESP, hdr.len);=0A break;=0A = }=0A =0A- default:=0A- if (msg_type =3D=3D XS_TRANSACTION_= START) {=0A- trans =3D kmalloc(sizeof(*trans), = GFP_KERNEL);=0A- if (!trans) {=0A- = rc =3D -ENOMEM;=0A- goto out;=0A- = }=0A+ case XS_TRANSACTION_START:=0A+ trans =3D = kmalloc(sizeof(*trans), GFP_KERNEL);=0A+ if (!trans) {=0A+ = rc =3D -ENOMEM;=0A+ goto out;=0A = }=0A+ goto common;=0A =0A+ case XS_TRANSACTION_END:=0A+ = list_for_each_entry(trans, &u->transactions, list)=0A+ if = (trans->handle.id =3D=3D u->u.msg.tx_id)=0A+ = break;=0A+ if (&trans->list =3D=3D &u->transactions) {=0A+ = rc =3D -ESRCH;=0A+ goto out;=0A+ = }=0A+ /* fall through */=0A+ common:=0A+ default:=0A = reply =3D xenbus_dev_request_and_reply(&u->u.msg);=0A if = (IS_ERR(reply)) {=0A- kfree(trans);=0A+ = if (msg_type =3D=3D XS_TRANSACTION_START)=0A+ = kfree(trans);=0A rc =3D PTR_ERR(reply);=0A = goto out;=0A }=0A@@ -309,21 +341,21 @@ static ssize_t = xenbus_dev_write(struct f=0A trans->handle.id =3D = simple_strtoul(reply, NULL, 0);=0A list_add(&trans->li= st, &u->transactions);=0A } else if (msg_type =3D=3D = XS_TRANSACTION_END) {=0A- list_for_each_entry(trans, = &u->transactions, list)=0A- if (trans->handle.i= d =3D=3D u->u.msg.tx_id)=0A- break;=0A- = BUG_ON(&trans->list =3D=3D &u->transactions);=0A = list_del(&trans->list);=0A kfree(trans);=0A = }=0A mutex_lock(&u->reply_mutex);=0A- = queue_reply(u, (char *)&u->u.msg, sizeof(u->u.msg));=0A- = queue_reply(u, (char *)reply, u->u.msg.len);=0A- mutex_unloc= k(&u->reply_mutex);=0A- kfree(reply);=0A+ err =3D = queue_reply(&queue, &u->u.msg, sizeof(u->u.msg))=0A+ ?: = queue_reply(&queue, reply, u->u.msg.len);=0A break;=0A = }=0A =0A+ queue_flush(u, &queue, err);=0A+ mutex_unlock(&u->re= ply_mutex);=0A+ kfree(reply);=0A+ if (err)=0A+ rc =3D = err;=0A+=0A out:=0A u->len =3D 0;=0A return rc;=0A --=__Part1834BD3E.0__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --=__Part1834BD3E.0__=--