On 17.02.25 13:09, Andrew Cooper wrote:
Hello Andrew
On 17/02/2025 10:27 am, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <[email protected]>
This is actually what the caller acquire_resource() expects on any kind
of error (the comment on top of resource_max_frames() also suggests that).
:(
So it broke somewhere between v3 and v8 of the original patch series,
but I can't seem to find the intervening series on lore.
Given the comment, I suspect I got inadvertently-reviewed into this bug.
Otherwise, the caller will treat -errno as a valid value and propagate incorrect
nr_frames to the VM. As a possible consequence, a VM trying to query a resource
size of an unknown type will get the success result from the hypercall and
obtain
nr_frames 4294967201.
This is one of the few interfaces we have low level testing for.
tools/tests/resource/test-resource.c
yes
Please could you add a step looking for an invalid resource id and check
you get 0 back?
Sure. I was thinking where to add this step and propose the following
change. I will send a formal patch once I find a way how to easily test
this change.
From 872565da55b7e87e1664714bb1b3ee84245db4a5 Mon Sep 17 00:00:00 2001
From: Oleksandr Tyshchenko <[email protected]>
Date: Mon, 17 Feb 2025 14:16:50 +0200
Subject: [PATCH] tests/resource: Verify that Xen can deal with invalid
resource type
The sign of the presence of a corresponding bugfix is an error
returned on querying the size of an unknown for Xen resource type.
Signed-off-by: Oleksandr Tyshchenko <[email protected]>
---
tools/tests/resource/test-resource.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/tests/resource/test-resource.c
b/tools/tests/resource/test-resource.c
index 1b10be16a6..197477c3f3 100644
--- a/tools/tests/resource/test-resource.c
+++ b/tools/tests/resource/test-resource.c
@@ -123,6 +123,22 @@ static void test_gnttab(uint32_t domid, unsigned
int nr_frames,
fail(" Fail: Managed to map gnttab v2 status frames in v1
mode\n");
xenforeignmemory_unmap_resource(fh, res);
}
+
+ /*
+ * While at it, verify that an attempt to query the size of an unknown
+ * for Xen resource type fails. Use the highest possible value for
variable
+ * of uint16_t type.
+ */
+ rc = xenforeignmemory_resource_size(
+ fh, domid, 65535,
+ XENMEM_resource_grant_table_id_shared, &size);
+
+ /*
+ * Success here indicates that Xen is missing the bugfix to make size
+ * requests return an error on an invalid resource type.
+ */
+ if ( !rc )
+ fail(" Fail: Expected error on an invalid resource type, got
success\n");
}
static void test_domain_configurations(void)
--
2.34.1
Also, add an ASSERT_UNREACHABLE() in the default case of _acquire_resource(),
normally we won't get to this point, as an unknown type will always be rejected
earlier in resource_max_frames().
Fixes: 9244528955de ("xen/memory: Fix acquire_resource size semantics")
Signed-off-by: Oleksandr Tyshchenko <[email protected]>
Reviewed-by: Jan Beulich <[email protected]>
~Andrew
