linux-kernel - [39/48] xen/xenbus: Reject replies with payload > XENSTORE_PAYLOAD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20120116184519.009780337@clark.kroah.org>
Date:	Mon, 16 Jan 2012 10:45:06 -0800
From:	Greg KH <gregkh@...e.de>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc:	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	alan@...rguk.ukuu.org.uk, Ian Campbell <ian.campbell@...rix.com>,
	Haogang Chen <haogangchen@...il.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Subject: [39/48] xen/xenbus: Reject replies with payload > XENSTORE_PAYLOAD_MAX.

3.1-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Ian Campbell <Ian.Campbell@...rix.com>

commit 9e7860cee18241633eddb36a4c34c7b61d8cecbc upstream.

Haogang Chen found out that:

 There is a potential integer overflow in process_msg() that could result
 in cross-domain attack.

 	body = kmalloc(msg->hdr.len + 1, GFP_NOIO | __GFP_HIGH);

 When a malicious guest passes 0xffffffff in msg->hdr.len, the subsequent
 call to xb_read() would write to a zero-length buffer.

 The other end of this connection is always the xenstore backend daemon
 so there is no guest (malicious or otherwise) which can do this. The
 xenstore daemon is a trusted component in the system.

 However this seem like a reasonable robustness improvement so we should
 have it.

And Ian when read the API docs found that:
        The payload length (len field of the header) is limited to 4096
        (XENSTORE_PAYLOAD_MAX) in both directions.  If a client exceeds the
        limit, its xenstored connection will be immediately killed by
        xenstored, which is usually catastrophic from the client's point of
        view.  Clients (particularly domains, which cannot just reconnect)
        should avoid this.

so this patch checks against that instead.

This also avoids a potential integer overflow pointed out by Haogang Chen.

Signed-off-by: Ian Campbell <ian.campbell@...rix.com>
Cc: Haogang Chen <haogangchen@...il.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>

---
 drivers/xen/xenbus/xenbus_xs.c     |    6 ++++++
 include/xen/interface/io/xs_wire.h |    3 +++
 2 files changed, 9 insertions(+)

--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -801,6 +801,12 @@ static int process_msg(void)
 		goto out;
 	}
 
+	if (msg->hdr.len > XENSTORE_PAYLOAD_MAX) {
+		kfree(msg);
+		err = -EINVAL;
+		goto out;
+	}
+
 	body = kmalloc(msg->hdr.len + 1, GFP_NOIO | __GFP_HIGH);
 	if (body == NULL) {
 		kfree(msg);
--- a/include/xen/interface/io/xs_wire.h
+++ b/include/xen/interface/io/xs_wire.h
@@ -84,4 +84,7 @@ struct xenstore_domain_interface {
     XENSTORE_RING_IDX rsp_cons, rsp_prod;
 };
 
+/* Violating this is very bad.  See docs/misc/xenstore.txt. */
+#define XENSTORE_PAYLOAD_MAX 4096
+
 #endif /* _XS_WIRE_H */


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/