tools/ocaml/xenstored: delete watch from trie too when resetting watches

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 834955fb084cae8a52e260c898a117c7a990fd1a..1a70d412d549c94b74b5f00e21d1aef238134a62 100644 (file)
--- 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 5e2347d085ae0505d0eab1ecf892f35cb2f4f0ed..e07508a2e4724b6647d2ee4cf04c22d37714d7a7 100644 (file)
--- 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                                                                                    *)