diff mbox series

[v1,2/4] tools/ocaml/xenstored: fix quota calculation for mkdir EEXIST

Message ID 6293bbc59bf4904e975dcc0e31608f858d34d84e.1610748224.git.edvin.torok@citrix.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Edwin Török Jan. 15, 2021, 10:29 p.m. UTC
We increment the domain's quota on mkdir even when the node already
exists.
This results in a quota inconsistency after live update, where
reconstructing the tree from scratch results in a different quota.

Not a security issue because the domain uses up quota faster,
so it will only get a Quota error sooner than it should.

Discovered by the structured fuzzing test:
```
live-update-agree: FAIL

When given the input:

  [{ "domid" = 0;
    "cmd" = { "tid" = 0;
              "rid" = 0;
              "op" = MKDIR;
              "data" = "/" } }; { "domid" = 0;
                                  "cmd" = { "tid" = 0;
                                            "rid" = 0;
                                            "op" = DEBUG;
                                            "data" = "live-update\000-s" } }]

the test failed:

    store agrement: diff --git 1/tmp/expected5b4372.txt 2/tmp/actual1c18b5.txt
index ac39964836..af318026ec 100644
diff mbox series

Patch

--- 1/tmp/expected5b4372.txt
+++ 2/tmp/actual1c18b5.txt
@@ -1,9 +1,9 @@ 
{ "stat_transaction_coalesce" = 0;
  "stat_transaction_abort" = 0;
  "store" = /{n0}
  /tool{n0}
  /local{n0}
  ;
  "quota" = { "maxent" = 8192;
  "maxsize" = 2048;
  "cur" = (hashtbl (0, +3+))-2-)) } }

Fatal error: exception Crowbar.TestFailure
```

This shows that the quota was 2 instead of 3 after a live update.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 tools/ocaml/xenstored/store.ml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml
index a9c079a417..1a9f71fa62 100644
--- a/tools/ocaml/xenstored/store.ml
+++ b/tools/ocaml/xenstored/store.ml
@@ -420,6 +420,7 @@  let mkdir store perm path =
 	(* It's upt to the mkdir logic to decide what to do with existing path *)
 	if not (existing || (Perms.Connection.is_dom0 perm)) then Quota.check store.quota owner 0;
 	store.root <- path_mkdir store perm path;
+	if not existing then
 	Quota.add_entry store.quota owner
 
 let rm store perm path =