Skip to content

Commit b3385ea

Browse files
ben-grandeben-grande
committed
Avoid reads to xenstore keys that are not written
Xenstore keys are created immediately after domain is crated and before tasks can be switched with "await", to guarantee that qubesd methods (same thread) don't fail attempts to read xenstore keys. This is the case for "Qubes.get_vm_stats", which interacts with VMM methods instead of qubes events.
1 parent 19da97d commit b3385ea

3 files changed

Lines changed: 63 additions & 43 deletions

File tree

‎qubes/app.py‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,27 @@ def query_stubdom_xid(domid) -> int:
584584
return int(stubdom_xid_str)
585585
return -1
586586

587+
def query_xs_memory(domid, key) -> int | None:
588+
if (
589+
f"xs_{key}" in current[domid]
590+
and current[domid][f"xs_{key}"] is False
591+
):
592+
return None
593+
untrusted_value = self.app.vmm.xs.read(
594+
"", f"/local/domain/{domid}/memory/{key}"
595+
)
596+
if untrusted_value is None:
597+
current[domid][f"xs_{key}"] = False
598+
del untrusted_value
599+
return None
600+
current[domid][f"xs_{key}"] = True
601+
if not untrusted_value.isdigit():
602+
del untrusted_value
603+
return None
604+
value = int(untrusted_value)
605+
del untrusted_value
606+
return value
607+
587608
info = sorted(info, key=lambda domain: domain["domid"])
588609
info = {v["domid"]: v for v in info}
589610
for domid in info.keys():
@@ -598,6 +619,12 @@ def query_stubdom_xid(domid) -> int:
598619
current[domid]["name"] = previous[domid]["name"]
599620
current[domid]["is_stubdom"] = previous[domid]["is_stubdom"]
600621
current[domid]["stubdom_xid"] = previous[domid]["stubdom_xid"]
622+
current[domid]["xs_meminfo"] = previous[domid].get(
623+
"xs_meminfo", None
624+
)
625+
current[domid]["xs_swapinfo"] = previous[domid].get(
626+
"xs_swapinfo", None
627+
)
601628
stubdom_xid = current[domid]["stubdom_xid"]
602629
elif "name" not in current[domid]:
603630
name = self.app.get_name_from_domid(domid)
@@ -637,32 +664,13 @@ def query_stubdom_xid(domid) -> int:
637664
]
638665

639666
if self.app.vmm.is_xen and not current[domid]["is_stubdom"]:
640-
untrusted_meminfo = self.app.vmm.xs.read(
641-
"", f"/local/domain/{domid}/memory/meminfo"
642-
)
643-
if (
644-
untrusted_meminfo is not None
645-
and untrusted_meminfo.isdigit()
646-
):
647-
meminfo = int(untrusted_meminfo)
648-
del untrusted_meminfo
667+
668+
meminfo = query_xs_memory(domid, "meminfo")
669+
if meminfo is not None:
649670
current[domid]["memory_with_swap_used"] = meminfo
650-
# Only query swapinfo is meminfo is valid to avoid
651-
# extraneous calls.
652-
untrusted_swapinfo = self.app.vmm.xs.read(
653-
"", f"/local/domain/{domid}/memory/swapinfo"
654-
)
655-
if (
656-
untrusted_swapinfo is not None
657-
and untrusted_swapinfo.isdigit()
658-
):
659-
swapinfo = int(untrusted_swapinfo)
660-
del untrusted_swapinfo
671+
swapinfo = query_xs_memory(domid, "swapinfo")
672+
if swapinfo is not None:
661673
current[domid]["swap_used"] = swapinfo
662-
else:
663-
del untrusted_swapinfo
664-
else:
665-
del untrusted_meminfo
666674

667675
current[domid]["online_vcpus"] = info[domid]["online_vcpus"]
668676
if stubdom_xid > 0:

‎qubes/tests/app.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,8 @@ def test_000_get_vm_stats_single(self, mock_qubesdb, mock_adminvm_qubesdb):
879879
"cpu_usage_raw": 0,
880880
"online_vcpus": 8,
881881
"stubdom_xid": -1,
882+
"xs_meminfo": True,
883+
"xs_swapinfo": False,
882884
},
883885
1: {
884886
"name": names[1],
@@ -893,6 +895,8 @@ def test_000_get_vm_stats_single(self, mock_qubesdb, mock_adminvm_qubesdb):
893895
"cpu_usage_raw": 0,
894896
"online_vcpus": 1,
895897
"stubdom_xid": -1,
898+
"xs_meminfo": True,
899+
"xs_swapinfo": True,
896900
},
897901
2: {
898902
"name": names[2],
@@ -907,6 +911,8 @@ def test_000_get_vm_stats_single(self, mock_qubesdb, mock_adminvm_qubesdb):
907911
"cpu_usage_raw": 0,
908912
"online_vcpus": 8,
909913
"stubdom_xid": -1,
914+
"xs_meminfo": True,
915+
"xs_swapinfo": True,
910916
},
911917
}
912918
self.assertEqual(info, expected_info)
@@ -925,7 +931,7 @@ def test_001_get_vm_stats_twice(self, mock_qubesdb, mock_adminvm_qubesdb):
925931
"/local/domain/1/memory/meminfo": b"849925",
926932
"/local/domain/2/memory/meminfo": b"849926",
927933
"/local/domain/0/memory/swapinfo": b"0",
928-
"/local/domain/1/memory/swapinfo": b"35",
934+
"/local/domain/1/memory/swapinfo": None,
929935
"/local/domain/2/memory/swapinfo": b"4",
930936
}
931937
self.app.get_name_from_domid = lambda domid: names[domid]
@@ -970,6 +976,8 @@ def test_001_get_vm_stats_twice(self, mock_qubesdb, mock_adminvm_qubesdb):
970976
"cpu_usage_raw": 80,
971977
"online_vcpus": 8,
972978
"stubdom_xid": -1,
979+
"xs_meminfo": True,
980+
"xs_swapinfo": True,
973981
},
974982
1: {
975983
"name": names[1],
@@ -978,12 +986,13 @@ def test_001_get_vm_stats_twice(self, mock_qubesdb, mock_adminvm_qubesdb):
978986
"memory_assigned_usable": 303916,
979987
"memory_kb": 303916,
980988
"memory_with_swap_used": 849925,
981-
"swap_used": 35,
982989
"cpu_time": 2849496569205,
983990
"cpu_usage": 100,
984991
"cpu_usage_raw": 100,
985992
"online_vcpus": 1,
986993
"stubdom_xid": -1,
994+
"xs_meminfo": True,
995+
"xs_swapinfo": False,
987996
},
988997
2: {
989998
"name": names[2],
@@ -998,6 +1007,8 @@ def test_001_get_vm_stats_twice(self, mock_qubesdb, mock_adminvm_qubesdb):
9981007
"cpu_usage_raw": 100,
9991008
"online_vcpus": 8,
10001009
"stubdom_xid": -1,
1010+
"xs_meminfo": True,
1011+
"xs_swapinfo": True,
10011012
},
10021013
}
10031014
self.assertEqual(info, expected_info)
@@ -1016,7 +1027,6 @@ def test_001_get_vm_stats_twice(self, mock_qubesdb, mock_adminvm_qubesdb):
10161027
("xs.read", ("", "/local/domain/0/memory/meminfo")),
10171028
("xs.read", ("", "/local/domain/0/memory/swapinfo")),
10181029
("xs.read", ("", "/local/domain/1/memory/meminfo")),
1019-
("xs.read", ("", "/local/domain/1/memory/swapinfo")),
10201030
("xs.read", ("", "/local/domain/2/memory/meminfo")),
10211031
("xs.read", ("", "/local/domain/2/memory/swapinfo")),
10221032
],

‎qubes/vm/qubesvm.py‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,6 +1496,7 @@ async def start(
14961496
self.libvirt_domain.createWithFlags(
14971497
libvirt.VIR_DOMAIN_START_PAUSED
14981498
)
1499+
self.create_xs_entries()
14991500

15001501
# the above allocates xid, lets announce that
15011502
self.fire_event("property-reset:xid", name="xid")
@@ -2811,6 +2812,23 @@ def relative_path(self, path):
28112812

28122813
return os.path.relpath(path, self.dir_path)
28132814

2815+
def create_xs_entries(self):
2816+
# TODO: Currently the whole qmemman is quite Xen-specific, so stay with
2817+
# xenstore for it until decided otherwise
2818+
if qmemman_present and self.maxmem:
2819+
xs_basedir = f"/local/domain/{self.xid}"
2820+
for key in ["meminfo", "swapinfo"]:
2821+
self.app.vmm.xs.write("", f"{xs_basedir}/memory/{key}", "")
2822+
self.app.vmm.xs.set_permissions(
2823+
"", f"{xs_basedir}/memory/{key}", [{"dom": self.xid}]
2824+
)
2825+
if self.use_memory_hotplug:
2826+
self.app.vmm.xs.write(
2827+
"",
2828+
f"{xs_basedir}/memory/hotplug-max",
2829+
str(self.maxmem * 1024),
2830+
)
2831+
28142832
def create_qdb_entries(self):
28152833
"""Create entries in Qubes DB."""
28162834
# pylint: disable=no-member
@@ -2876,22 +2894,6 @@ def create_qdb_entries(self):
28762894
self.untrusted_qdb.write("/qubes-block-devices", "")
28772895
self.untrusted_qdb.write("/qubes-usb-devices", "")
28782896

2879-
# TODO: Currently the whole qmemman is quite Xen-specific, so stay with
2880-
# xenstore for it until decided otherwise
2881-
if qmemman_present and self.maxmem:
2882-
xs_basedir = f"/local/domain/{self.xid}"
2883-
for key in ["meminfo", "swapinfo"]:
2884-
self.app.vmm.xs.write("", f"{xs_basedir}/memory/{key}", "")
2885-
self.app.vmm.xs.set_permissions(
2886-
"", f"{xs_basedir}/memory/{key}", [{"dom": self.xid}]
2887-
)
2888-
if self.use_memory_hotplug:
2889-
self.app.vmm.xs.write(
2890-
"",
2891-
f"{xs_basedir}/memory/hotplug-max",
2892-
str(self.maxmem * 1024),
2893-
)
2894-
28952897
self.fire_event("domain-qdb-create")
28962898

28972899
@qubes.events.handler("property-pre-set:maxmem")

0 commit comments

Comments
 (0)