From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= <edvin.torok@citrix.com>
Subject: tools/ocaml/xenstored: delete watch from trie too when resetting
 watches
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

c/s f8c72b526129 "oxenstored: implement XS_RESET_WATCHES" from Xen 4.6
introduced reset watches support in oxenstored by mirroring the change
in cxenstored.

However the OCaml version has some additional data structures to
optimize watch firing, and just resetting the watches in one of the data
structures creates a security bug where a malicious guest kernel can
exceed its watch quota, driving oxenstored into OOM:
 * create watches
 * reset watches (this still keeps the watches lingering in another data
   structure, using memory)
 * create some more watches
 * loop until oxenstored dies

The guest kernel doesn't necessarily have to be malicious to trigger
this:
 * if control/platform-feature-xs_reset_watches is set
 * the guest kexecs (e.g. because it crashes)
 * on boot more watches are set up
 * this will slowly "leak" memory for watches in oxenstored, driving it
   towards OOM.

This is XSA-330.

Fixes: f8c72b526129 ("oxenstored: implement XS_RESET_WATCHES")
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/tools/ocaml/xenstored/connections.ml b/tools/ocaml/xenstored/connections.ml
index 9f9f7ee2f0..6ee3552ec2 100644
--- a/tools/ocaml/xenstored/connections.ml
+++ b/tools/ocaml/xenstored/connections.ml
@@ -134,6 +134,10 @@ let del_watch cons con path token =
 		cons.watches <- Trie.set cons.watches key watches;
  	watch
 
+let del_watches cons con =
+	Connection.del_watches con;
+	cons.watches <- Trie.map (del_watches_of_con con) cons.watches
+
 (* path is absolute *)
 let fire_watches ?oldroot root cons path recurse =
 	let key = key_of_path path in
diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index 73e04cc18b..437d2dcf9e 100644
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -179,8 +179,8 @@ let do_isintroduced con _t domains _cons data =
 	if domid = Define.domid_self || Domains.exist domains domid then "T\000" else "F\000"
 
 (* only in xen >= 4.2 *)
-let do_reset_watches con _t _domains _cons _data =
-  Connection.del_watches con;
+let do_reset_watches con _t _domains cons _data =
+  Connections.del_watches cons con;
   Connection.del_transactions con
 
 (* only in >= xen3.3                                                                                    *)