summaryrefslogtreecommitdiff
blob: 5fc3c7746c909ba4524e0be554f937eca3e02e4c (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
From 635390415f4a9c0621330f0b40f8c7e914c4523f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= <edvin.torok@citrix.com>
Date: Wed, 12 Oct 2022 19:13:05 +0100
Subject: [PATCH 83/87] tools/ocaml: Ensure packet size is never negative
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Integers in Ocaml have 63 or 31 bits of signed precision.

On 64-bit builds of Ocaml, this is fine because a C uint32_t always fits
within a 63-bit signed integer.

In 32-bit builds of Ocaml, this goes wrong.  The C uint32_t is truncated
first (loses the top bit), then has a unsigned/signed mismatch.

A "negative" value (i.e. a packet on the ring of between 1G and 2G in size)
will trigger an exception later in Bytes.make in xb.ml, and because the packet
is not removed from the ring, the exception re-triggers on every subsequent
query, creating a livelock.

Fix both the source of the exception in Xb, and as defence in depth, mark the
domain as bad for any Invalid_argument exceptions to avoid the risk of
livelock.

This is XSA-420 / CVE-2022-42324.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit ae34df4d82636f4c82700b447ea2c93b9f82b3f3)
---
 tools/ocaml/libs/xb/partial.ml   | 6 +++---
 tools/ocaml/xenstored/process.ml | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/ocaml/libs/xb/partial.ml b/tools/ocaml/libs/xb/partial.ml
index b6e2a716e263..3aa8927eb7f0 100644
--- a/tools/ocaml/libs/xb/partial.ml
+++ b/tools/ocaml/libs/xb/partial.ml
@@ -36,7 +36,7 @@ let of_string s =
 	   This will leave the guest connection is a bad state and will
 	   be hard to recover from without restarting the connection
 	   (ie rebooting the guest) *)
-	let dlen = min xenstore_payload_max dlen in
+	let dlen = max 0 (min xenstore_payload_max dlen) in
 	{
 		tid = tid;
 		rid = rid;
@@ -46,8 +46,8 @@ let of_string s =
 	}
 
 let append pkt s sz =
-	if pkt.len > 4096 then failwith "Buffer.add: cannot grow buffer";
-	Buffer.add_string pkt.buf (String.sub s 0 sz)
+	if Buffer.length pkt.buf + sz > xenstore_payload_max then failwith "Buffer.add: cannot grow buffer";
+	Buffer.add_substring pkt.buf s 0 sz
 
 let to_complete pkt =
 	pkt.len - (Buffer.length pkt.buf)
diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index ce39ce28b5f3..6cb990ee7fb2 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -722,7 +722,7 @@ let do_input store cons doms con =
 			History.reconnect con;
 			info "%s reconnection complete" (Connection.get_domstr con);
 			None
-		| Failure exp ->
+		| Invalid_argument exp | Failure exp ->
 			error "caught exception %s" exp;
 			error "got a bad client %s" (sprintf "%-8s" (Connection.get_domstr con));
 			Connection.mark_as_bad con;
-- 
2.37.4