From d4b884bf35f46f3b5efc925847d0144c3e30dee7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Edwin=20T=C3=B6r=C3=B6k?= Date: Tue, 15 Dec 2020 14:30:16 +0100 Subject: [PATCH] tools/ocaml/xenstored: delete watch from trie too when resetting watches MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 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 Acked-by: Christian Lindig Reviewed-by: Andrew Cooper --- tools/ocaml/xenstored/connections.ml | 4 ++++ tools/ocaml/xenstored/process.ml | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/ocaml/xenstored/connections.ml b/tools/ocaml/xenstored/connections.ml index 834955fb08..1a70d412d5 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 5e2347d085..e07508a2e4 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 *) -- 2.39.5