[xen staging-4.16] tools/ocaml/xenstored: Fix quota bypass on domain shutdown

[patchbot@xxxxxxx]

commit 5b0919f2c0e5060f6e0bc328f100abae0a9f07b8
Author:     Edwin Török <edvin.torok@xxxxxxxxxx>
AuthorDate: Wed Oct 12 19:13:06 2022 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Nov 1 14:07:24 2022 +0000

    tools/ocaml/xenstored: Fix quota bypass on domain shutdown
    
    XSA-322 fixed a domid reuse vulnerability by assigning Dom0 as the owner of
    any nodes left after a domain is shutdown (e.g. outside its /local/domain/N
    tree).
    
    However Dom0 has no quota on purpose, so this opened up another potential
    attack vector. Avoid it by deleting these nodes instead of assigning them to
    Dom0.
    
    This is part of XSA-419 / CVE-2022-42323.
    
    Fixes: c46eff921209 ("tools/ocaml/xenstored: clean up permissions for dead 
domains")
    Signed-off-by: Edwin Török <edvin.torok@xxxxxxxxxx>
    Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx>
    (cherry picked from commit db471408edd46af403b8bd44d180a928ad7fbb80)
---
 tools/ocaml/xenstored/perms.ml |  3 +--
 tools/ocaml/xenstored/store.ml | 29 +++++++++++++++++++++--------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/tools/ocaml/xenstored/perms.ml b/tools/ocaml/xenstored/perms.ml
index e8a16221f8..84f2503e8e 100644
--- a/tools/ocaml/xenstored/perms.ml
+++ b/tools/ocaml/xenstored/perms.ml
@@ -64,8 +64,7 @@ let get_owner perm = perm.owner
 * *)
 let remove_domid ~domid perm =
        let acl = List.filter (fun (acl_domid, _) -> acl_domid <> domid) 
perm.acl in
-       let owner = if perm.owner = domid then 0 else perm.owner in
-       { perm with acl; owner }
+       if perm.owner = domid then None else Some { perm with acl; owner = 
perm.owner }
 
 let default0 = create 0 NONE []
 
diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml
index 20e67b1427..70f0c83de4 100644
--- a/tools/ocaml/xenstored/store.ml
+++ b/tools/ocaml/xenstored/store.ml
@@ -87,10 +87,21 @@ let check_owner node connection =
 
 let rec recurse fct node = fct node; SymbolMap.iter (fun _ -> recurse fct) 
node.children
 
-(** [recurse_map f tree] applies [f] on each node in the tree recursively *)
-let recurse_map f =
+(** [recurse_filter_map f tree] applies [f] on each node in the tree 
recursively,
+    possibly removing some nodes.
+    Note that the nodes removed this way won't generate watch events.
+*)
+let recurse_filter_map f =
+       let invalid = -1 in
+       let is_valid _ node = node.perms.owner <> invalid in
        let rec walk node =
-               f { node with children = SymbolMap.map walk node.children }
+               (* Map.filter_map is Ocaml 4.11+ only *)
+               let node =
+               { node with children =
+                       SymbolMap.map walk node.children |> SymbolMap.filter 
is_valid } in
+               match f node with
+               | Some keep -> keep
+               | None -> { node with perms = {node.perms with owner = invalid 
} }
        in
        walk
 
@@ -444,11 +455,13 @@ let setperms store perm path nperms =
 
 let reset_permissions store domid =
        Logging.info "store|node" "Cleaning up xenstore ACLs for domid %d" 
domid;
-       store.root <- Node.recurse_map (fun node ->
-               let perms = Perms.Node.remove_domid ~domid node.perms in
-               if perms <> node.perms then
-                       Logging.debug "store|node" "Changed permissions for 
node %s" (Node.get_name node);
-               { node with perms }
+       store.root <- Node.recurse_filter_map (fun node ->
+               match Perms.Node.remove_domid ~domid node.perms with
+               | None -> None
+               | Some perms ->
+                       if perms <> node.perms then
+                               Logging.debug "store|node" "Changed permissions 
for node %s" (Node.get_name node);
+                       Some { node with perms }
        ) store.root
 
 type ops = {
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.16