[PATCH] xen: vm_event: do not do vm_event_op for an invalid domain

[PATCH] xen: vm_event: do not do vm_event_op for an invalid domain

[Volodymyr Babchuk]

A privileged domain can issue XEN_DOMCTL_vm_event_op with
op->domain == DOMID_INVALID. In this case vm_event_domctl()
function will get NULL as the first parameter and this will
cause hypervisor panic, as it tries to derefer this pointer.

Fix the issue by checking if valid domain is passed in.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

---

This issue was found by the xen fuzzer ([1])

[1] https://lore.kernel.org/all/20250315003544.1101488-1-volodymyr_babchuk@epam.com/
---
 xen/common/vm_event.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index fbf1aa0848..a4c233de52 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -600,6 +600,13 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec)
         return 0;
     }
 
+    if ( unlikely(!d) )
+    {
+        gdprintk(XENLOG_INFO,
+                 "Tried to do a memory event op on invalid domain\n");
+        return -EINVAL;
+    }
+
     rc = xsm_vm_event_control(XSM_PRIV, d, vec->mode, vec->op);
     if ( rc )
         return rc;
-- 
2.48.1

Re: [PATCH] xen: vm_event: do not do vm_event_op for an invalid domain

[Andrew Cooper]

On 17/03/2025 11:08 pm, Volodymyr Babchuk wrote:
> A privileged domain can issue XEN_DOMCTL_vm_event_op with
> op->domain == DOMID_INVALID. In this case vm_event_domctl()
> function will get NULL as the first parameter and this will
> cause hypervisor panic, as it tries to derefer this pointer.
>
> Fix the issue by checking if valid domain is passed in.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>
> ---
>
> This issue was found by the xen fuzzer ([1])
>
> [1] https://lore.kernel.org/all/20250315003544.1101488-1-volodymyr_babchuk@epam.com/
> ---
>  xen/common/vm_event.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index fbf1aa0848..a4c233de52 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -600,6 +600,13 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec)
>          return 0;
>      }
>  
> +    if ( unlikely(!d) )
> +    {
> +        gdprintk(XENLOG_INFO,
> +                 "Tried to do a memory event op on invalid domain\n");
> +        return -EINVAL;
> +    }
> +
>      rc = xsm_vm_event_control(XSM_PRIV, d, vec->mode, vec->op);
>      if ( rc )
>          return rc;

Oops.  Git blame says this is my fault.

This behaviour is intentional.  See XEN_DOMCTL_vm_event_op (along with
test_assign_device and get_domain_state) early in do_domctl().

It was introduced in commit d48e1836074c ("vm_event: Add a new opcode to
get VM_EVENT_INTERFACE_VERSION") so that XEN_VM_EVENT_GET_VERSION could
succeed.

Apparently I deleted it in commit 48b84249459f ("xen/vm-event: Drop
unused u_domctl parameter from vm_event_domctl()"), and that wasn't
intentional.

That will want putting in with an extra comment.

/* All other subops need to target a real domain. */
if ( unlikely(d == NULL) )
    return -ESRCH;

Don't both with a printk().  It's just noise, and -ESRCH is correct code
to use.

~Andrew

Re: [PATCH] xen: vm_event: do not do vm_event_op for an invalid domain

[Tamas K Lengyel]

On Mon, Mar 17, 2025 at 7:08 PM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> A privileged domain can issue XEN_DOMCTL_vm_event_op with
> op->domain == DOMID_INVALID. In this case vm_event_domctl()
> function will get NULL as the first parameter and this will
> cause hypervisor panic, as it tries to derefer this pointer.
>
> Fix the issue by checking if valid domain is passed in.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>
> ---
>
> This issue was found by the xen fuzzer ([1])
>
> [1] https://lore.kernel.org/all/20250315003544.1101488-1-volodymyr_babchuk@epam.com/
> ---
>  xen/common/vm_event.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index fbf1aa0848..a4c233de52 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -600,6 +600,13 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec)
>          return 0;
>      }
>
> +    if ( unlikely(!d) )
> +    {
> +        gdprintk(XENLOG_INFO,
> +                 "Tried to do a memory event op on invalid domain\n");

This is not a memory event op?

Tamas

Re: [PATCH] xen: vm_event: do not do vm_event_op for an invalid domain

[Volodymyr Babchuk]

Hi Tamas,


Tamas K Lengyel <tamas@tklengyel.com> writes:

> On Mon, Mar 17, 2025 at 7:08 PM Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>> A privileged domain can issue XEN_DOMCTL_vm_event_op with
>> op->domain == DOMID_INVALID. In this case vm_event_domctl()
>> function will get NULL as the first parameter and this will
>> cause hypervisor panic, as it tries to derefer this pointer.
>>
>> Fix the issue by checking if valid domain is passed in.
>>
>> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>>
>> ---
>>
>> This issue was found by the xen fuzzer ([1])
>>
>> [1] https://lore.kernel.org/all/20250315003544.1101488-1-volodymyr_babchuk@epam.com/
>> ---
>>  xen/common/vm_event.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
>> index fbf1aa0848..a4c233de52 100644
>> --- a/xen/common/vm_event.c
>> +++ b/xen/common/vm_event.c
>> @@ -600,6 +600,13 @@ int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec)
>>          return 0;
>>      }
>>
>> +    if ( unlikely(!d) )
>> +    {
>> +        gdprintk(XENLOG_INFO,
>> +                 "Tried to do a memory event op on invalid domain\n");
>
> This is not a memory event op?

Oh, this is good catch. I absent mindedly copied an error message from a
couple of lines below. Looks like we need another patch that fixes error
messages.


--
WBR, Volodymyr