diff mbox series

[V3,for-4.20] xen/memory: Make resource_max_frames() to return 0 on unknown type

Message ID 20250217223402.167514-1-olekstysh@gmail.com (mailing list archive)
State New
Headers show
Series [V3,for-4.20] xen/memory: Make resource_max_frames() to return 0 on unknown type | expand

Commit Message

Oleksandr Tyshchenko Feb. 17, 2025, 10:34 p.m. UTC 
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

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).
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.

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().

Also, update test-resource app to verify that Xen can deal with invalid
(unknown) resource type properly.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Refer:
https://patchew.org/Xen/20250217102741.4122367-1-olekstysh@gmail.com/
https://patchew.org/Xen/20250217204822.136437-1-olekstysh@gmail.com/

  V2:
   - add R-b
   - add ASSERT_UNREACHABLE() in the default case of _acquire_resource()
     and update commit desc regarding that
   - drop post-commit remark

  V3:
   - merge with the change for tools/tests/resource/test-resource.c
     and update commit desc regarding that
---
---
 tools/tests/resource/test-resource.c | 10 ++++++++++
 xen/common/memory.c                  |  3 ++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Andrew Cooper Feb. 17, 2025, 10:41 p.m. UTC | #1 
On 17/02/2025 10:34 pm, Oleksandr Tyshchenko wrote:
> diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
> index 1b10be16a6..521c1fc51a 100644
> --- a/tools/tests/resource/test-resource.c
> +++ b/tools/tests/resource/test-resource.c
> @@ -123,6 +123,16 @@ 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);
>      }
> +
> +    /*
> +     * If this check starts failing, you've find the right place to test your

s/find/found/

Can fix on commit, if Oleksii is happy for this to go into 4.20.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Oleksii Kurochko Feb. 18, 2025, 8:28 a.m. UTC | #2 
On 2/17/25 11:41 PM, Andrew Cooper wrote:
> On 17/02/2025 10:34 pm, Oleksandr Tyshchenko wrote:
>> diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
>> index 1b10be16a6..521c1fc51a 100644
>> --- a/tools/tests/resource/test-resource.c
>> +++ b/tools/tests/resource/test-resource.c
>> @@ -123,6 +123,16 @@ 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);
>>       }
>> +
>> +    /*
>> +     * If this check starts failing, you've find the right place to test your
> s/find/found/
>
> Can fix on commit, if Oleksii is happy for this to go into 4.20.
>
> Reviewed-by: Andrew Cooper<andrew.cooper3@citrix.com>

The fix looks simply and low risk so lets take it into 4.20:
   Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii
diff mbox series

Patch

diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
index 1b10be16a6..521c1fc51a 100644
--- a/tools/tests/resource/test-resource.c
+++ b/tools/tests/resource/test-resource.c
@@ -123,6 +123,16 @@  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);
     }
+
+    /*
+     * If this check starts failing, you've find the right place to test your
+     * addition to the Acquire Resource infrastructure.
+     */
+    rc = xenforeignmemory_resource_size(fh, domid, 3, 0, &size);
+
+    /* Check that Xen rejected the resource type. */
+    if ( !rc )
+        fail("    Fail: Expected error on an invalid resource type, got success\n");
 }
 
 static void test_domain_configurations(void)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index a6f2f6d1b3..8ca4e1a842 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1157,7 +1157,7 @@  static unsigned int resource_max_frames(const struct domain *d,
         return d->vmtrace_size >> PAGE_SHIFT;
 
     default:
-        return -EOPNOTSUPP;
+        return 0;
     }
 }
 
@@ -1240,6 +1240,7 @@  static int _acquire_resource(
         return acquire_vmtrace_buf(d, id, frame, nr_frames, mfn_list);
 
     default:
+        ASSERT_UNREACHABLE();
         return -EOPNOTSUPP;
     }
 }