summaryrefslogtreecommitdiff
blob: bc741ae629f0bf9ce1f9e8b32024a17280f3f031 (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
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
From 45816222bb3da04f4cd3388efc46d127d48b8906 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= <edvin.torok@citrix.com>
Date: Thu, 28 Jul 2022 17:08:15 +0100
Subject: [PATCH 102/126] tools/ocaml/xenstored: Check for maxrequests before
 performing operations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously we'd perform the operation, record the updated tree in the
transaction record, then try to insert a watchop path and the reply packet.

If we exceeded max requests we would've returned EQUOTA, but still:
* have performed the operation on the transaction's tree
* have recorded the watchop, making this queue effectively unbounded

It is better if we check whether we'd have room to store the operation before
performing the transaction, and raise EQUOTA there.  Then the transaction
record won't grow.

This is part of XSA-326 / CVE-2022-42317.

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

diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index 27790d4a5c41..dd58e6979cf9 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -389,6 +389,7 @@ let input_handle_error ~cons ~doms ~fct ~con ~t ~req =
 	let reply_error e =
 		Packet.Error e in
 	try
+		Transaction.check_quota_exn ~perm:(Connection.get_perm con) t;
 		fct con t doms cons req.Packet.data
 	with
 	| Define.Invalid_path          -> reply_error "EINVAL"
@@ -681,9 +682,10 @@ let process_packet ~store ~cons ~doms ~con ~req =
 		in
 
 		let response = try
+			Transaction.check_quota_exn ~perm:(Connection.get_perm con) t;
 			if tid <> Transaction.none then
 				(* Remember the request and response for this operation in case we need to replay the transaction *)
-				Transaction.add_operation ~perm:(Connection.get_perm con) t req response;
+				Transaction.add_operation t req response;
 			response
 		with Quota.Limit_reached ->
 			Packet.Error "EQUOTA"
diff --git a/tools/ocaml/xenstored/transaction.ml b/tools/ocaml/xenstored/transaction.ml
index 17b1bdf2eaf9..294143e2335b 100644
--- a/tools/ocaml/xenstored/transaction.ml
+++ b/tools/ocaml/xenstored/transaction.ml
@@ -85,6 +85,7 @@ type t = {
 	oldroot: Store.Node.t;
 	mutable paths: (Xenbus.Xb.Op.operation * Store.Path.t) list;
 	mutable operations: (Packet.request * Packet.response) list;
+	mutable quota_reached: bool;
 	mutable read_lowpath: Store.Path.t option;
 	mutable write_lowpath: Store.Path.t option;
 }
@@ -127,6 +128,7 @@ let make ?(internal=false) id store =
 		oldroot = Store.get_root store;
 		paths = [];
 		operations = [];
+		quota_reached = false;
 		read_lowpath = None;
 		write_lowpath = None;
 	} in
@@ -143,13 +145,19 @@ let get_root t = Store.get_root t.store
 
 let is_read_only t = t.paths = []
 let add_wop t ty path = t.paths <- (ty, path) :: t.paths
-let add_operation ~perm t request response =
+let get_operations t = List.rev t.operations
+
+let check_quota_exn ~perm t =
 	if !Define.maxrequests >= 0
 		&& not (Perms.Connection.is_dom0 perm)
-		&& List.length t.operations >= !Define.maxrequests
-		then raise Quota.Limit_reached;
+		&& (t.quota_reached || List.length t.operations >= !Define.maxrequests)
+		then begin
+			t.quota_reached <- true;
+			raise Quota.Limit_reached;
+		end
+
+let add_operation t request response =
 	t.operations <- (request, response) :: t.operations
-let get_operations t = List.rev t.operations
 let set_read_lowpath t path = t.read_lowpath <- get_lowest path t.read_lowpath
 let set_write_lowpath t path = t.write_lowpath <- get_lowest path t.write_lowpath
 
-- 
2.37.4